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