Safe call to user input based function

I am trying to create an AJAX script that will take two GET variables, a class and a method, and map them to the methods we developed (similar to how CodeIgniter works for ajax, I'm sure). Since I rely on user input to determine which class and method to execute, I am worried that the hacker might use some method to his 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 flaws or safety notes that I should be aware of when using this technique?

+6
source share
5 answers
 <?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 other answers for best results. Requires PHP 5.3.0 or later. You can 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 that are very verbose (easy to maintain) and do not require you to maintain an array from which classes can be called and which cannot.

+6
source

Check if the method is allowed to the 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 cannot control what the user can do.

+14
source

Given that you are not passing any arguments, this is relatively safe. But I would add a list of valid classes to your IF, for example:

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

Thus, a hacker cannot really name any possible class in the structure in this way, but only those that you allow him to.

+4
source

In this case, you must handle Reflection .

Here is an 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); ?> 

The execution of the method can be like ReflectionMethods: invoke :

  <?php class HelloWorld { public function sayHelloTo($name) { return 'Hello ' . $name; } } $reflectionMethod = new ReflectionMethod('HelloWorld', 'sayHelloTo'); echo $reflectionMethod->invoke(new HelloWorld(), 'Mike'); ?> 

So 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'); } 
+2
source

urlencode () excites me a bit. Although it would probably be safe, I would have a more rigorous understanding. I would only allow letters, numbers, and underscores. In fact, you do not need class or method names with other characters. I do not think I've ever seen.

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

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

And, as mentioned in other posters, you definitely need some kind of whitelist to explicitly allow (for classes, at least since I'm sure that not every class should be accessible from ajax). We also check class_exists () and method_exists ().

I would also recommend a message type reminder system if any of these checks failed. I am sure you would like to know if someone is trying hax0r j00.

+1
source

Source: https://habr.com/ru/post/920123/


All Articles