2

I recently stumbled across the following in our codebase:

$func = $_GET['func'] . '_xyz123';
if(function_exists($func)){
   $result = $func($_GET);
   echo(json_encode($result));
}

It makes me wonder, if it would be possible to pass in a php builtin function + some garbage to $_GET['func'], so that it would cancel out _xyz123 causing an RCE exploit.

Is this possible, or am I just being paranoid?

Ahmed Akhtar
  • 1,444
  • 1
  • 16
  • 28
jjb
  • 77
  • 2
  • 9
  • A null character might do it, but that makes me wonder why they would bother. it does seem like a really odd way to build an exploit, but then "odd" is the way that a lot of exploits go undetected. – samlev Feb 19 '16 at 19:27
  • A \0 character won't do: https://3v4l.org/6vsmJ – VolkerK Feb 19 '16 at 19:52
  • Neither does any other character in the unicode range 0x0...0xFFFF: https://3v4l.org/3MlSg – trincot Feb 19 '16 at 19:59
  • Whether or not this particular case is exploitable, I think you're right to be concerned -- running a function based on a name built from a GET parameter is a code smell in my book. At the very least, one could alter the namespace by sending a string like `\Foo\bar` – Alex Howansky Feb 19 '16 at 20:37
  • Thanks @AlexHowansky, I didn't even think of that possibility. – jjb Feb 19 '16 at 21:03

1 Answers1

2

It is indeed risky to leave that code as it stands.

I tried to find a character that could prematurely clip a string. But although I found no way to do that, this is not conclusive, and there could be other ways to tamper with function names. As PHP offers namespace syntax (using \), that also could be exploited by creative users.

But there is no reason to take the risk and allow user input to contain characters that should not occur in function names. Function names must follow rules:

Function names follow the same rules as other labels in PHP. A valid function name starts with a letter or underscore, followed by any number of letters, numbers, or underscores. As a regular expression, it would be expressed thus: [a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*

So, you could make sure any argument that has a violating character is rejected (including the backslash in namespace syntax). Using the quoted regular expression you could do that as follows:

function isValidName($name) {
    return preg_match("/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/", $name);
}

$func = $_GET['func'] . '_xyz123';

if(isValidName($func) && function_exists($func)){
    $result = $func($_GET);
    echo(json_encode($result));
}

An additional way to secure the application, is to define all functions that can be called this way as static methods of a class. Let's assume that class is called _xyz123, and your functions would not have this suffix, then the code could look like this, using method_exists:

$func = $_GET['func'];
if(isValidName($func) && method_exists('_xyz123', $func)){

Or you could define all those functions in a specific namespace, e.g. _xyz123, and do this:

if(isValidName($func) && function_exists('\\_xyz123\\$func')){
trincot
  • 317,000
  • 35
  • 244
  • 286