Checking PHP security with PHP

The checker

There are many potential properties of a PHP project that one might want to verify, but the particular case which motivated this blog post was a switch from PHP’s mysql and mysqli raw query functions to its prepared statement functions, in a project I was working on. The first approach one might try is to grep the source code for mentions of mysql_query or mysqli_query, but in this project (most of) the actual uses of those functions were nicely encapsulated in a DB class. So, grepping could at least find instances where the DB class was not used, and they could be ported over to using it, but then the task becomes one of detecting instances of where the DB class is being used. This is not simply a matter of removing the deprecated methods on the old DB class and seeing what breaks, however, as unfortunately the old functionality had to be kept around for use by some legacy code (which is not exposed to users).

The idea for the improved checker, therefore, was to also look for instances of loading the DB class, so that files which were not entitled to use it could be transitioned to the new preparedStatementDB class. This meant looking for occurrences of new DB(); as the DB class could be loaded through class autoloading, without having to require() or include() any files. As an extra precaution, though, one could also check the names of the files being required or included by grepping for any mention of these functions. Because of inheritance though, someone could create a subclassDB, so there would have to be a check for extends DB, just in case.

One final class of security assurances we would want is that no one had used prepared statements in an unsafe way, such as using string manipulation to build the queries that are then prepared. This could in theory be done by checking that the mysqli_stmt_prepare() command was not called with an argument containing a dollar. Although someone might want to legitimately set a value in the database containing a dollar, they could do that by binding the string they want to a parameter of a prepared statement string. Thus with a bit of care, it is possible to code any desired functionality in PHP using this programmatically auditable subset of the language.

The attacks

Of course, if one is trying to write software to check for deliberate security vulnerabilities introduced by rogue contributors, perhaps one has a bigger problem than the choice of programming language, but I like to think that if you aim for stopping deliberate errors, you end up catching a lot of accidental ones you otherwise would have missed. The sort of accidental error in the code someone might make, which would thwart the automated checker, is inclusion of excessive whitespace (possibly even new lines) between the “new” and the “DB” in new DB();, although that should be easy to take into account in a parser. Then there is the possible addition of comments, although if someone wants to write new /* This is a legitimate comment */ DB(); then you might as well have the checker flag that up whatever the specific class is they are trying to load.

Where it gets really tricky is when people start doing new $foo(); having assigned the string “DB” to the variable $foo. One could mandate that the subset of PHP being used forbids loading variably named classes like that, except this feature of PHP can be useful when using the factory pattern, which is a perfectly legitimate thing to want to do. If the auditor took the alternate approach of searching for string variables with the value “DB” then this quickly becomes the sort of cat and mouse game that the attacker always wins.

Unfortunately there is a wider class of run-time generated code that PHP will execute, which means casting the net even wider to catch the most sneaky coder. For example, PHP has an eval() function, although this should probably be checked for and rejected anyway, even though “this can be useful for storing code in a database text field for later execution.”, supposedly. There is also the slightly more legitimate create_function() which would have to be restricted from source files, although with PHP 5.3 this function is effectively obsolete as anonymous closure-style functions can be used instead, which do not provide an opportunity for malicious coders to hide references to forbidden functions.

Conclusion

So, it seems almost possible to write a regular expression based checker of PHP source files that checks if the code conforms to some enhanced-security subset of PHP. Most PHP coders probably don’t understand the factory pattern anyway, and if they did you could possibly wrap up the dangerous new $className(); code in some safe, audited, whitelisted function. Getting PHP coders to adapt to this subset of the language might be a bit harder, but it might not be that difficult if there were a standard security tool which popular PHP projects used and gave them a way of saying “We are RegExpAuditor compliant.” By being a standard tool, it could be the subject of security analysis by the community and improved as people found ways around it.

But would using a proper language like Java with its loadClass() method make code easier to audit?