2011
08.18

Summary

A PHP application that displays error reporting notices and contains specific code patterns may be vulnerable to a cross-site scripting attack. I’ve confirmed this issue on PHP 5.2.17 and a snapshot of PHP 5.4 (I assume it affects other versions of PHP as well). This issue was filed as Sec Bug #55139 back in July, but it was recently closed as “bogus” by a member of the PHP team, making the report public.

How does it work?

When display_errors is enabled and a PHP notice is generated, none of the text of the notice is HTML-encoded. That means if an attacker can control part of the notice text, they can inject arbitrary HTML and JavaScript into the page. Certain specific coding patterns make such an attack possible.

For example, the “Undefined index” notice does not properly sanitize the name of the index. That means an application containing code similar to the following would be vulnerable to cross-site scripting:

1
2
3
4
<?php  
error_reporting(E_ALL);
$array = array();
echo $array[$_GET['index']];

You can test this yourself: upload that script and try browsing to file.php?index=<s>test</s>. You’ll see strikethrough text instead of the literal string <s>test</s>.

This also holds true for less common coding patterns and notices. For example:

1
2
3
4
5
<?php
// Undefined function. this is included for completeness:
// if you have code that does this, you have much larger problems
error_reporting(E_ALL);
echo $_GET['funct']();

1
2
3
4
<?php
// undefined variable
error_reporting(E_ALL);
echo $$_GET['var'];

1
2
3
4
5
<?php
// defining a constant twice
error_reporting(E_ALL);
define($_GET['cons'], 'a');
define($_GET['cons'], 'a');

But display_errors needs to be enabled!

Yes, display_errors needs to be enabled. Yes, that means your server is potentially leaking sensitive information (including absolute paths). Yes, that’s not something you should be doing on a production server. But under normal circumstances, that’s the extent of any security issues. A sysadmin or a developer who makes the decision to enable display_errors has no expectation that doing so also (potentially) opens up their site to cross-site scripting.

Lets also not forget that display_errors may be enabled in development environments. If an attacker can identify a vulnerable page and can figure out the URL for a working development environment, the attacker can send that URL, modified to exploit the cross-site scripting vulnerability, to a developer. If it gets clicked on, the development environment can become compromised.

Does code like that exist in the real world?

Yes! ;-)

As a proof of concept, I identified a location in Wordpress (/wp-admin/upload.php) containing a vulnerable code pattern. It looked like:

1
2
3
4
5
<?php
if ( isset($_GET['message']) && (int) $_GET['message'] ) {
   $message = $messages[$_GET['message']];
   $_SERVER['REQUEST_URI'] = remove_query_arg(array('message'), $_SERVER['REQUEST_URI']);
}

In that case, an attacker could supply $_GET['message'] as 1<script>alert(1)</script>, triggering an alert box.

This issue would not be feasible to exploit on most installations: Wordpress explicitly excludes E_NOTICE from error_reporting unless a special debug mode is enabled. However, the code has been updated in SVN and is no longer vulnerable. I want to extend my thanks to the Wordpress developers for addressing this issue. :-)

Does it only affect PHP notices?

Under certain situations, no!

Greg asked in the comments what the impact of the html_errors INI option is. As it turns out, when that option is disabled, examples that I had previously tested become vulnerable. Previously, only notices appeared unsanitized. When that option is disabled, every message (warnings, fatal errors, etc) appears to end up unsanitized. Very good finding. :)

Conclusion

Don’t enable display_errors in production: it can now cause cross-site scripting as well as information disclosure.

Comments