PHP Security Vulnerability, Unvalidated Post data used for object instantiation

Question

I'm looking at some basic PHP code that takes a string from POST data via an ajax call. The POST data is used to directly instantiate a class whose name is equivalent to the value of the POSTed string. Assuming that all the class autoloader does is throw an error or an exception if it can't find a class, what harm could a malicious user inflict here?

POSTing a standard PHP class name that requires parameters, or POSTing 'stdClass' wouldn't do much other than prevent a return view. Beyond that, what could be possible? Furthermore, what would be the best way to validate and to still be flexible when new view classes are added.

require_once('autoloader.php');

if(!empty($_POST['viewRequst'])){

    try{       

        $requestedView = $_POST['viewRequest'];

        $view = new $requestedView();

        $view->outputPage();

    }catch(Exception $e){

        /** 
        Perform Some Logging
        **/

    }

}else{

    echo "Data Error"; 

}

Whitelist the possible acceptable values for `$_POST['viewRequest']` would be a good additional safety check — Mark Baker, Feb 25 '15 at 08:21
I assume this will trigger a heap of answers about the security implications. At the _absolute minimum_ use a convention-over-configuration approach like `$reqView = 'Request'.$_POST['viewRequest'].'View';`. But as mentioned in the previous comment a white list approach would be better ...far better. — VolkerK, Feb 25 '15 at 08:23
I was thinking of writing a class that would scan the directory that holds the view classes to generate a whitelist. That way I wouldn't have to manually maintain the whitelist. While it would be a flexible way of maintaining a list, I'm not sure if that would qualify as a "best practice" since it would basically run every time a new view post request comes in. — RoJG, Feb 25 '15 at 08:39
@RoJG: that's not a bad idea, but do it in your build process, or a local post-commit hook, rather than at run-time. — halfer, Feb 25 '15 at 09:09

score 1 · Answer 1 · answered Feb 25 '15 at 12:31

You probably don't want to create arbitrary objects based on user data because this leads to Object Injection.

I suggest a whitelist for the names of the classes, e.g.

/* a global function or part of a library */
function getValidPostClassName($str)
{
    switch($str) {
        case 'Something':
        case 'SomethingElse':
            return $str;
        default:
            throw new Exception("Security error!");
    }
}

$requestedView = getValidPostClassName($_POST['viewRequest']);
$view = new $requestedView();

I also, personally, would not design my applications to ever give user variables any control over object instantiation.

PHP Security Vulnerability, Unvalidated Post data used for object instantiation

1 Answers1