How do I avoid reflection injection attacks in PHP?

Question

I'm writing a class that allows you to bridge HTTP requests with class instances using JSON for data, without any implementation in the class you're bridging to. Basically this is how it works:

// This is just an ordinary class.
$service = new WeatherService();

$jhi = new JsonHttpInterface($service);
$jhi->exec();

The JsonHttpInterface class will check the PATH_INFO of the request and call that method, applying any query string parameters as arguments.

http://example.com/the_above.php/getWeather?state="CA" would translate to
$service->getWeather("CA") (assuming that the name of the first argument is $state).

This is how the method is found and called:

$method = new ReflectionMethod(get_class($this->instance), $action);
/*
... code that matches query string values to arguments of above method...
*/
$response = $method->invokeArgs($this->instance, $args);

Now what I'm wondering is: what are the vulnerabilities of such a system. I've been pretty lenient on error checking, relying on PHP to throw errors when attempting to call non-existent or private/protected methods.

• Is it possible to cheat the system?
• Is it possible to pass in an invalid method name that does something other than throw an error?
• Is it possible to refer to a method in a base class, or any other class?

The full source of JsonHttpInterface is available here: http://blixt.org/js/two-cents.php

Answer 1

Anthing that allows user-entered PHP code is likely to be open to some exploitation, so it's certainly high risk. If you do keep going with it then one check I would consider including is making any classes that you want accessed in this way implement a particular interface and check for the interface before executing. That would prevent any arbitrary class from being executed in that way and just restrict it to those that you've specifically allowed.

A better solution might be to to auto-generate static files with all the calls in that you need, that way you've explictly allowed particular actions rather than trying to second-guess where all the holes might be.

Answer 2

First of all,

You need to create a white_list.

You get the defined functions of the object, and you check to see if the function name sent to you is in the array, if it is, good, else, forget about it.

You can either do an automatic whitelist, or a manual one that is configured, your choice.

You need to create a munging/washing function for all passed values.

In the example of a state, state can be CA, or California, so essentially, it must preg_match('/\w+/') so you can do some easy checking to verify data.

If you wanted to get fancy, you could allow people to create a method called allowedValues which is a hash map of functionName => /Regex/, and use that as a valid data lookup table, if the function isn't defined in the class, you use a generic one that only allows string/numbers and perhops only of a certain length.

    class MyClass{

    public function allowedArguments() {
        //This acts as a white list, and a cleaner/restricter
        return array(
            'myfunc' => array(
                'pattern' => '/\w+/'
                'length' => 255
            )
        );
    }

    public function myFunc($arg) {
        ... do something
    }
}

Answer 3

You can achieve the same without using the ReflectionXYZ classes

call_user_func( array($this->instance, $action) , $args);

And both are save in the same way, as long as you control what $this->instance is.
$action is a string that is used to search an entry in the object's/class' method hashtable and there's no magic symbol that can escape the object context (switching to another object). And there's no parsing involved like e.g. in sql and sql injections.
Both ReflectionMethod and call_user_func_array() honour the protection level of methods. e.g.

class Foo {
  public function publicfn() {
    echo 'abc';
  }

  protected function protectedfn() {
    echo 'xyz';
  }
}

$obj = new Foo;
call_user_func_array(array($obj, 'publicfn'), array());
call_user_func_array(array($obj, 'protectedfn'), array());
$ro = new ReflectionMethod($obj, 'protectedfn');
$ro->invokeArgs($obj, array());

prints

abc
Warning: call_user_func_array() expects parameter 1 to be a valid callback, cannot access protected method Foo::protectedfn() in blabla on line 14

Fatal error: Uncaught exception 'ReflectionException' with message 'Trying to invoke protected method Foo::protectedfn() from scope ReflectionMethod' in blabla:16
Stack trace:
#0 blabla(16): ReflectionMethod->invokeArgs(Object(Foo), Array)
#1 {main}
  thrown in blabla on line 16

You might want to look up if this was always the case. There's e.g. an entry - MFH Fixed bug #37816 (ReflectionProperty does not throw exception when accessing protected attribute). At least it's true for php 5.3 branch.
You can always access public methods of any base class of $this->instance.
You can access protected methods from within the class context, i.e. if $this and $this->instance are of the same/a derived type protected methods of $this->instance are accessible. e.g.

class Foo {
  protected $instance;
  public function __construct(Foo $instance=null) {
    $this->instance = $instance;
  }
  public function publicfn() {
    if ( !is_null($this->instance)) {
      call_user_func_array( array($this->instance, 'protectedfn'), array());
    }
  }

  protected function protectedfn() {
    echo 'Foo::protectedfn() invoked';
  }
}

class Bar extends Foo {
  protected function protectedfn() {
    echo 'Bar::protectedfn() invoked';
  }
}

$foo = new Foo(new Bar);
$foo->publicfn();

prints Bar::protectedfn() invoked. But that shouldn't be too hard to avoid.