Pale Purple

Insecure PHP Code?

palepurple

Recently on the PHP NorthWest mailing list, there has been some discussion over the causes of security vulnerabilities within PHP applications.

The original poster provided a questionnaire – however, this didn’t really allow for much in the way of a detailed answer, so here’s our 2 pence. As a caveat: there are obviously other causes…. :

Lack of Awareness

Some developers just aren’t aware of, or don’t care about, security – as a sweeping generalisation they probably won’t count themselves as a professional programmer. For example: someone who was originally a web designer and learns enough PHP to process a form and slowly moves onto bigger things. Such people are likely to be copying and pasting code which has been butchered over time to add in new fields and functionality. Their understanding of the code and language is often minimal (which is possible due to PHP’s low barrier to entry).

Limited Knowledge

More professional/full time developers will be aware of (for example) SQL injections – but, in some cases, only because a user had typed in “O’Reilly” or > in a form and it resulted in an error occurring – and an error report being raised to them. The developer is likely to have solved the problem by littering the code-base with calls to functions like addslashes() or htmlentities() – however, this can result in the data being encoded/escaped multiple times (so you might see “it\\\\\\\\\\\\\\’s” or &ampgt;).

Over-reliance on trust

Other times developers may believe they can trust the end user – phases like the following are likely heard in this scenario :

Unfortunately a reliance on trust will probably lead to authentication bypass, cross site scripting and possibly data disclosure – for example, not ensuring a user is logged in before allowing for the deletion/modification of data, or not checking that someone is authorised to view an invoice before showing it.

Poor application design

In other circumstances, a developer will just have a horrible ‘foundation’ to work with – whether they wrote or inherited a code-base with fundamental design issues which make it hard to develop in a secure manner going forward. As an illustration, something as simple as the following PHP code where the original author assumed the caller would always escape/sanitise/validate the inputs before using them –

function add_user($sanitised_login, $sanitised_name, $sanitised_password) {
    // probably missing length and other validity checks here.
    mysql_query("INSERT INTO users (login, name, password) 
                 VALUES ('$sanitised_login', '$sanitised_name', '$sanitised_password')");
}

And at some point, the function is called and the inputs will not have been appropriately validated or sanitised – like :


add_user($_POST['x'], .....).

This is clearly a case of bad design – possibly coupled with commercial pressures which stop the developer from going back and refactoring it when the problems become apparent.

Commercial Pressures

It’s rare to find a customer who states security on a requirements specification. Likewise, being given the resources to perform a pro-active audit of a code-base is unlikely to gain approval from many managers when the time spent on an audit could be used to add additional functionality – which may result in a greater number of sales. Most organisations are focussed on short-term results, so refactoring a code-base, or introducing a testing suite which could take weeks or months to perform, is likely to be difficult to introduce – even with well documented long term benefits.

We frequently help sites to become PCI DSS compliant (which generally involves a combination of systems administration and code review/modification) – in this instance there has been a clear commercial pressure to be secure – but certainly the code review/patching part is at the wrong stage of the development cycle.

The less obvious vulnerabilities

There are security vulnerabilities which are harder to spot, such as session fixation, race conditions and symlink attacks.

Some of these are dealt with through following best practice and developer experience (for example: knowing they should not write out to a hard coded file path like /tmp/tmpfile.txt and should use tempnam()/tmpfile() instead).

Others may be through use of a framework (where the framework calls session_regenerate_id() when someone authenticates, avoiding session fixation) or perhaps it passes view content through the HtmlPurifier library before it’s rendered to reduce the chance of Cross Site Scripting. Alternatively, perhaps it uses a database abstraction layer like Doctrine or Propel which makes it effectively immune to SQL injection.

Suffice to say, in our PHP Security training course we cover various design specific items which should help the above.

Everyone makes mistakes

We recently reviewed a PHP app written in the Yii framework (which seems to be quite a good framework). However, the developers had introduced an XSS vulnerability in a custom 404 error page they’d added – an unescaped error message was being output to the page (which contained part of the URL).

The Yii framework obviously contains functionality to protect an end user from such issues – for example using CHtml::encode($error) would have stopped http://whatever/blah/<img src="http://lolwut.com/layout/lolwut.jpg" alt="" /> from working as well.

Which leads into ….

Detection and Risk-reduction

So, although use of a framework will give a project a good foundation and hopefully reduce the scope for vulnerabilities, like everything, it is not a magic bullet and will not necessarily make the software immune to vulnerabilities. A “determined” developer can still introduce vulnerabilities. And obviously people sometimes make mistakes.

We didn’t realise one of our customers had an XSS vulnerability in code we’d written for them until we pointed an automated tool at the search form, and it’s for exactly this reason that we include practicals covering the use of a number of scanning tools within our PHP security training course.

Exit mobile version