6

PHP lets you instantiate classes from variables or array entries, like this:

class Foo {}
$className = 'Foo';
new $className();
$someArray = ['class_name' => 'Foo'];
new $someArray['class_name']();

Distressingly, some of my coworkers - on multiple projects I've worked on at multiple companies - have used this language feature with class names supplied from user input as a way of instantiating one of several possible subclasses depending upon a type specified in the request. I've seen code along these lines...

new $_GET['product_type']($_GET['product_id']);

This, besides being a maintenance headache, is obviously stupid and dangerous; you're letting an attacker instantiate an arbitrary class. But how dangerous? What attacks are there that just use built-in PHP classes (and therefore require no detailed knowledge of the application code) that could be used against endpoints like the one above?

I'm looking for the nastiest attack anyone can come up with that I can use to scare any future coworkers who use this horrible anti-pattern.

  • 1
    @MarkAmery if you could give a better example as to something your coworkers would do we would be able to provide a better counter example as to why allowing arbitrary class instantiation is bad. – d0nut Jan 26 '16 at 15:31
  • 2
    @silverpenguin I don't see how you could inject arbitrary code here? – Mark Amery Jan 26 '16 at 15:44
  • 1
    Well, from what you said in your earlier comment about unchecked JSON, this kind of unchecked and unverified input is always a security vulnerability. But I can't see the rest of the code, so I can't show you if there are others. Is it correct that the built-in classes and functions are never instantiated by the coder? If that is the case, I don't think you'd be able to exploit it this way. – Mark Buffalo Jan 26 '16 at 15:56
  • 1
    @MarkBuffalo You're definitely misunderstanding the mechanics of this. The way the code here works is that the client passes a class name to the server, and the server (assuming, but not validating, that the class name is one of a few that it expects) instantiates the class with the given name. This allows the client to make the server instantiate any PHP class it likes, as long as it's built into PHP or already defined somewhere in the serverside application. The question is how that can be practically exploited, without a deep knowledge of the application. – Mark Amery Jan 26 '16 at 16:00
  • 1
    What happens if you have a constructor with side effects? Allowing a user to execute those side effects can certainly cause badness to occur. – Neil Smithline Jan 26 '16 at 16:12
2

I don't think that you can do anything with just this code.

But that does not mean that this is secure in any way. You give up the control flow of your application to the user, which is not a good idea.

For example, if you were to call close on the newly created object, an attacker would gain SSRF:

$obj = new $_GET['x']($_GET['y']);
$obj->close();

// SSRF: 
http://localhost/shell.php?x=SoapClient&y=http://localhost/called

Or if you had a class such as this, an attacker would gain session fixation:

class Session {
   function __construct($sessionId) {
       session_id($sessionId);
   }
}

// session fixation:    
http://localhost/shell.php?x=Session&y=123

Or you may have a class like this:

class File {
    function __construct($filename, $content) {
        $this->filename = $filename;
        $this->content = $content;
    }

    // [...]

    function save() {
        file_put_contents($this->filename, $this->content);
    }
}

And lets say the classes that users are supposed to create also have a save method (eg to save the object to the database). Now an attacker would have code execution.

It's easy to imagine a lot more scenarios like this, where either a constructor of a class does something dangerous, or a dangerous method of a class has the same name as a safe method of the class you expect.

It's likely that your current code base would allow an attacker to do at least something.

Depending on your code being secret is also not a good idea. Your code should be secure, even if the source code somehow leaked.

  • 1
    I like your last paragraph. – Mark Buffalo Jan 26 '16 at 16:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.