Question

I would like to allow small user-defined regular expressions to be submitted for testing. However, there are many problems to consider from run-away server usage to more evil eval() usage.

To my knowledge I have handled all the problems I could think of in the following code. Are their any attack vectors I haven't thought of? (A rather naive question I know)

function testRegex($regex)
{
    // null character allows a premature regex end and "/../e" injection
    if (strpos($regex, 0) !== false || ! trim($regex)) {
        return false;
    }

    $backtrack_limit = ini_set('pcre.backtrack_limit', 200);
    $recursion_limit = ini_set('pcre.recursion_limit', 20);

    $valid = @preg_match("~$regex~u", null) !== false;

    ini_set('pcre.backtrack_limit', $backtrack_limit);
    ini_set('pcre.recursion_limit', $recursion_limit);

    return $valid;
}


$regexes = array(
    "InvalidRegular)Expression",
    '',
    '\w+',
    '\/\w+/',
    'foo[bar]*',
    '\/\x00known/e' . chr(0x00) . chr(0),
    'known~e' . chr(0),
    'known~e' . chr(0x00),
    '[a-z]+',
    '\p{Lu}+',
);


foreach($regexes as $regex) {
    var_dump($regex, testRegex($regex));
}

If you want to see an example of a null-byte injection:

$user_regex = '.+~e' . chr(0);
$user_match = 'system("whoami")';

var_dump(preg_replace("~$user_regex~u", $user_match, 'foo'));
Était-ce utile?

La solution

Obviously, the only way to test whether a string is a valid regular expression is by compiling it (which is done when you call any of the matching functions), so what you're doing makes a lot of sense.

The null-byte protection you have added is actually not necessary since 5.4, because there are already checks made in the leader, the middle and the ending. The latter in particular is a relatively recent commit (2011) to fix this bug.

Setting a lower backtrack and recursion limit is a good enough sandbox, perhaps you could check for a maximum length as well.

That said, this particular solution doesn't provide the ability to use modifiers such as /s, /i and /m; perhaps that's not your main concern at the moment, but rather food for thought :)

Autres conseils

You could tell them to input something like {expression} and then use preg_replace(). That way, they only use what you let them.

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top