Safely calling a function based off user input

Question

I'm trying to create an AJAX script that will take two GET variables, class and method, and map them to methods we've designed (similar to how CodeIgniter acts for ajax, I'm pretty sure). Since I'm relying on user input to determine what class and method to execute I'm worried that there may be some way for a hacker to use that technique to their advantage.

The code:

//Grab and clean (just in case, why not) the class and method variables from GET
$class = urlencode(trim($_GET['c']));
$method = urlencode(trim($_GET['m']));

//Ensure the passed function is callable
if(method_exists($class, $method)){
    $class::$method();
}

Are there any disadvantages or security watch-outs I should be aware of while using this technique?

Mariusz Jamro · Accepted Answer · 2012-07-10 13:32:22Z

14

Check if method is allowed to be called by user:

// methods that user can call:
$user_methods = array("method1", "method2", "method3", );

//Ensure the passed function is callable
if(method_exists($class, $method) and in_array($method, $user_methods){
    $class::$method();
}

Otherwise you'll have no control what user will be able to do.

answered Jul 10, 2012 at 13:32

Mariusz Jamro

31.9k25 gold badges129 silver badges170 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Erik Over a year ago

good answer; using method_exists alone can still cause unexpected things to happy.

Brandon - Free Palestine · Accepted Answer · 2012-07-10 19:34:49Z

6

<?php
class AjaxCallableFunction
{
    public static $callable_from_ajax = TRUE;
}

$class = $_POST['class'];
$method = $_POST['method'];

if ( class_exists( $class ) && isset( $class::$callable_from_ajax ) && $class::$callable_from_ajax ) {
    call_user_func( $class, $method );
}

Combine with some of the other answers for best results. Requires PHP 5.3.0 or higher. You could even implement an interface

<?php
interface AjaxCallable {}

class MyClass implements AjaxCallable 
{
    // Your code here
}

$class = $_POST['class'];
$method = $_POST['method'];

if ( class_exists( $class ) && in_array( 'AjaxCallable', class_implements( $class ) ) ) {
    call_user_func( $class, $method );
}

This approach follows OOP principles, is very verbose (Easy to maintain) and doesn't require you to maintain an array of which classes can be called and which cannot.

edited Jul 10, 2012 at 19:34

answered Jul 10, 2012 at 19:29

Brandon - Free Palestine

16.7k12 gold badges60 silver badges92 bronze badges

1 Comment

ACobbs Over a year ago

That AjaxCallable interface was a great idea, works perfectly. Thanks!

Mathieu Dumoulin · Accepted Answer · 2012-07-10 13:30:24Z

4

Considering you are not passing any arguments, this is relatively safe for now. But i would add a list of valid classes in your IF such as:

//Ensure the passed function is callable
if(method_exists($class, $method)){
    if(in_array($class, array('controller1', 'controller2'))){
        $class::$method();
    }
}

This way, a hacker can't really call any possible class in the framework this way but only the ones you allow him to.

answered Jul 10, 2012 at 13:30

Mathieu Dumoulin

12.3k7 gold badges47 silver badges71 bronze badges

Comments

Rosmarine Popcorn · Accepted Answer · 2012-07-10 14:45:20Z

2

You have to handle with Reflection in this case .

Here's and example of what you need.

<?php
class Apple {
    public function firstMethod() { }
    final protected function secondMethod() { }
    private static function thirdMethod() { }
}

$class = new ReflectionClass('Apple');
$methods = $class->getMethods();
var_dump($methods);
?>

Executing a method could be like this using ReflectionMethods:invoke:

  <?php
class HelloWorld {

    public function sayHelloTo($name) {
        return 'Hello ' . $name;
    }

}

$reflectionMethod = new ReflectionMethod('HelloWorld', 'sayHelloTo');
echo $reflectionMethod->invoke(new HelloWorld(), 'Mike');
?>

So finally we could :

 $class = urlencode(trim($_GET['c']));
  $method = urlencode(trim($_GET['m']));

  $allowed_methods = array("insert", "update", "delete");

  if(method_exists($class, $method) and in_array($method, $allowed_methods){
    $reflectionMethod = new ReflectionMethod($class, $method);
    $reflectionMethod->invoke(new $class, 'First Argument');
   }

edited Jul 10, 2012 at 14:45

answered Jul 10, 2012 at 13:50

Rosmarine Popcorn

11k11 gold badges62 silver badges90 bronze badges

4 Comments

ACobbs Over a year ago

What benefits does using Reflection offer over just calling the method directly?

Rosmarine Popcorn Over a year ago

@ACobbs im not sure if you call a method from string first ,next thing is that you wont mismatch the reference if someone attacks you're site. It will call that method using it's reference to the class where it belongs .

Theodore R. Smith Over a year ago

@Cody Could you please add an array for "allowed functions" and basically include Secator's answer in your own? Then your's will be undeniably the only answer I can vote for.

Theodore R. Smith Over a year ago

+1 This is the only safe and wise method in this entire thread.

LaVache · Accepted Answer · 2012-07-13 09:52:17Z

The urlencode() makes me a bit worried. Even though it might possibly be safe, I would sanitize much more strictly. I'd only allow letters, numbers and underscores. You shouldn't really need any class or method names with other characters. I don't think I've ever seen any.

I use this for A LOT of stuff in all my projects:

function very_safe_string( $string )
{
    return preg_replace("/[^A-Za-z0-9_]/" , '' , $string);
}

And as other posters have mentioned, you should definitely have some type of white list explicitly allowing (for the classes at least, as I'm sure not every class needs to be accessed from ajax). As well as checking class_exists() and method_exists().

I'd also recommend some type of email alert system in place if any of these checks fail. I'm sure you'd like to know if somebody is trying to hax0r j00.

Collectives™ on Stack Overflow

Safely calling a function based off user input

5 Answers 5

1 Comment

1 Comment

Comments

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

1 Comment

1 Comment

Comments

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related