Common PHP Regular Expression Security Issue

Stefan Esser (PHP Security Blog, Suhosin) recently posted an entry on his blog titled “Holes in most preg_match() filters” about a possible security issue that apparently escapes a lot of notice.

Let me explain the situation.  PHP uses Perl Compatible Regular Expressions, PCRE, for pattern matching.  In PCRE the carat metacharacter (^) is used to match the very beginning of the string, and the dollar-sign metacharacter ($) is used to match the end of the string.  This is extremely useful to ensure that the expression you’ve written has matched the entire string.

However, PCRE_DOLLAR_ENDONLY is not used by default.  This means that the dollar-sign metacharacter still matches to the end of the string, but it also matches is a newline character is at the end of the string.  In other words, a newline character may, or may not be present at the end of the string and you won’t know either way by default.

So, how do we fix this then?  Well, there are two ways.  First, you can add a D modifier to the end of the regular expression like this :

preg_match(‘/^[a-z]+$/D’, $string);

Or, you can use the \z modifier like this :

preg_match(‘/^[a-z]+\z/’, $string);

Either method works, although from the comments at Stefan’s site, it looks like \z is more portable since Perl doesn’t support the D modifier.

Here is short script to “prove” this, as it were :

 

$badstring = urldecode(“test%0a”);

if (preg_match(‘/^[0-9a-zA-Z]+$/’, $badstring)) {

print “Test 1 MATCHES\n”

}

if (preg_match(‘/^[0-9a-zA-Z]+$/D’, $badstring)) {

print “Test 2 MATCHES\n”

}

if (preg_match(‘/^[0-9a-zA-Z]+\z/’, $badstring)) {

print “Test 3 MATCHES\n”

}

 

I’m posting this info for two reasons.  First, it’s something programmers need to know.  It’s important since security holes are a bad thing.  Second, I’m guilty of this myself.  phpTodo used the dollar-sign metacharacter without the D modifier, making my code somewhat insecure.

The good news is that I have corrected the problem and posted a new version.  This is a precautionary measure, I don’t believe this adversely affected the security of the application, but better safe than sorry.  Head over and grab the new version just to be on the safe side.

Leave a Reply

Your email address will not be published. Required fields are marked *