5 ms·

Cross-Site Scripting? In PHP Notices? It's more likely than you think

43 points by nbpoole 15y ago

hartror 15y ago
As the OP points out this shouldn't be an issue on production systems as display_errors shouldn't be turned on. From the PHP docs [1]: This is a feature to support your development and should never be used on production systems (e.g. systems connected to the internet). Of course how many of the notoriously lazy/incompetent PHP developers we have all had to deal with in our lives have RTFM. So could PHP perhaps HTML encode error messages? Would this break any error message use cases? For example are there times when error messages appear when they wouldn't be interpreted as HTML encoded? CLI? Browsers loading different text mime types? [1] http://www.php.net/manual/en/errorfunc.configuration.php#ini.display-errors http://www.php.net/manual/en/errorfunc.configuration.php#ini...
- nbpoole 15y ago
 > So could PHP perhaps HTML encode error messages? Would this break any error message use cases? For example are there times when error messages appear when they wouldn't be interpreted as HTML encoded? CLI? Browsers loading different text mime types? This is something PHP already gets right with warnings, fatal errors, etc. The issue is only with notices as far as I've seen (I haven't delved into the PHP source to see what the difference is). A demonstration file: [cmd]$ cat 147b9119e818c92f7f74bad71cc12255.php <?php echo file_get_contents('<s>test</s>'); Compare: [cmd]$ php -f 147b9119e818c92f7f74bad71cc12255.php Warning: file_get_contents(<s>test</s>): failed to open stream: No such file or directory in /home/nbsandbox/sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.php on line 2 to http://sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.php http://sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.ph... Warning: file_get_contents(<s>test</s>) [<a href='function.file-get-contents'>function.file-get-contents</a>]: failed to open stream: No such file or directory in /home/nbsandbox/sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.php on line 2 
 - georgefox 15y ago
 I know what you mean, but the link you provided doesn't actually seem to be doing any HTML encoding.
 - nbpoole 15y ago
 Haha, whoops! Sorry about that, back to normal. Anyone who hit that URL in the past 10 minutes would have seen unsanitized HTML, because I was testing the impact of disabling the html_errors INI option (something one of my blog's comments asked about). I've updated the blog post to discuss html_errors as well. ;-)
pilif 15y ago
So with this I can XSS myself while in development mode on my development system? That's handy. There are so many good reasons to turn off display_error during production use, one of which is to not have a broken page due to errors appearing all over the place which even the laziest PHP developer who only cares about "stuff working" would appreciate. That it also leaks important internal information is another side-effect which generally isn't as appreciated though. Also the article is showing this code: // Undefined function error_reporting(E_ALL); echo $_GET['funct'](); which they admit is a "rarely used pattern". I would certainly hope so as it is also only marginally better than eval($_GET['data']); or just echo "the root password for this machine is: pAsswoRd";
- nbpoole 15y ago
 > So with this I can XSS myself while in development mode on my development system? That's handy. Or someone can send you a URL that points to your dev server and contains a payload. Or someone can send a URL to the developer across the street who uses shared hosting where display_errors is enabled by default. I'm not arguing that this is a feasible attack to pull off against most applications: it most certainly is not. I'm arguing that it shouldn't be possible to pull it off at all, even in the small percentage of applications that might be affected. There is no good reason PHP should be displaying this information unsanitized. --- You're 100% correct about the example you're pointing out. If you're allowing arbitrary functions called from the user, you're in much bigger trouble. ;-) I originally noticed and reported the issue with the first example, which triggers a simple, common undefined index notice: <?php error_reporting(E_ALL); $array = array(); echo $array[$_GET['index']]; I only included the others for the sake of completeness. I've updated the code sample to note that.
 - pilif 15y ago
 > Or someone can send you a URL that points to your dev server and contains a payload. That is theoretically possible, though what they would be able to do is to deal damage to my development install (if I don't to CSRF protection on POST operations) or they could potentially hijack my current session on my development system, which, too I don't deem too critical. > who uses shared hosting where display_errors is enabled by default init_set('display_errors', 0); Even on a shared host you can turn display_errors off. > There is no good reason PHP should be displaying this information unsanitized. I agree, especially because they already do some escaping of their own errors (and consequently don't escape while running in the CLI SAPI)
j_col 15y ago
I think if I was dumb enough to leave display_error on in production, I'd have a bigger problem with crackers being able to figure out sensitive details about my system from said error messages than worrying about them injecting scripts into them. Not saying that I have not seen this in the wild, but I have zero sympathy for those that don't carry out these basic steps to secure their production servers.
dave1010uk 15y ago
This is an example of CWE-81: "Improper Neutralization of Script in an Error Message Web Page" [1]. For those of you working with WordPress, this is the code you need in wp-config.php to log errors but not show them in web pages: define('WP_DEBUG', true); define('WP_DEBUG_DISPLAY', false); define('WP_DEBUG_LOG', true); [1] http://cwe.mitre.org/data/definitions/81.html http://cwe.mitre.org/data/definitions/81.html
deleted 15y ago
[deleted]
smcj 15y ago
Their bug tracker has a "Bogus" value? WTF? Well, PHP devs at its best: uninformed, ignorant, arrogant.
veloper 15y ago
This really falls under a few important coding mantras... Never trust the user. Never assume. Don't do stupid shit.
randomdrake 15y ago
I agree with many of the issues raised here and had my own issues with the blog post. I wrote up a response which contains more acceptable conclusions/fixes to the issues you present. http://randomdrake.com/2011/08/18/no-php-errors-and-notices-do-not-need-to-be-encoded/ http://randomdrake.com/2011/08/18/no-php-errors-and-notices-...
- nbpoole 15y ago
  Thanks, I just responded on your blog.

43 points by nbpoole 15y ago

As the OP points out this shouldn't be an issue on production systems as display_errors shouldn't be turned on. From the PHP docs [1]: This is a feature to support your development and should never be used on production systems (e.g. systems connected to the internet). Of course how many of the notoriously lazy/incompetent PHP developers we have all had to deal with in our lives have RTFM. So could PHP perhaps HTML encode error messages? Would this break any error message use cases? For example are there times when error messages appear when they wouldn't be interpreted as HTML encoded? CLI? Browsers loading different text mime types? [1] http://www.php.net/manual/en/errorfunc.configuration.php#ini.display-errors http://www.php.net/manual/en/errorfunc.configuration.php#ini...

nbpoole 15y ago
> So could PHP perhaps HTML encode error messages? Would this break any error message use cases? For example are there times when error messages appear when they wouldn't be interpreted as HTML encoded? CLI? Browsers loading different text mime types? This is something PHP already gets right with warnings, fatal errors, etc. The issue is only with notices as far as I've seen (I haven't delved into the PHP source to see what the difference is). A demonstration file: [cmd]$ cat 147b9119e818c92f7f74bad71cc12255.php <?php echo file_get_contents('<s>test</s>'); Compare: [cmd]$ php -f 147b9119e818c92f7f74bad71cc12255.php Warning: file_get_contents(<s>test</s>): failed to open stream: No such file or directory in /home/nbsandbox/sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.php on line 2 to http://sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.php http://sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.ph... Warning: file_get_contents(<s>test</s>) [<a href='function.file-get-contents'>function.file-get-contents</a>]: failed to open stream: No such file or directory in /home/nbsandbox/sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.php on line 2 
- georgefox 15y ago
 I know what you mean, but the link you provided doesn't actually seem to be doing any HTML encoding.
 - nbpoole 15y ago
 Haha, whoops! Sorry about that, back to normal. Anyone who hit that URL in the past 10 minutes would have seen unsanitized HTML, because I was testing the impact of disabling the html_errors INI option (something one of my blog's comments asked about). I've updated the blog post to discuss html_errors as well. ;-)

pilif 15y ago

So with this I can XSS myself while in development mode on my development system? That's handy. There are so many good reasons to turn off display_error during production use, one of which is to not have a broken page due to errors appearing all over the place which even the laziest PHP developer who only cares about "stuff working" would appreciate. That it also leaks important internal information is another side-effect which generally isn't as appreciated though. Also the article is showing this code: // Undefined function error_reporting(E_ALL); echo $_GET['funct'](); which they admit is a "rarely used pattern". I would certainly hope so as it is also only marginally better than eval($_GET['data']); or just echo "the root password for this machine is: pAsswoRd";

nbpoole 15y ago
> So with this I can XSS myself while in development mode on my development system? That's handy. Or someone can send you a URL that points to your dev server and contains a payload. Or someone can send a URL to the developer across the street who uses shared hosting where display_errors is enabled by default. I'm not arguing that this is a feasible attack to pull off against most applications: it most certainly is not. I'm arguing that it shouldn't be possible to pull it off at all, even in the small percentage of applications that might be affected. There is no good reason PHP should be displaying this information unsanitized. --- You're 100% correct about the example you're pointing out. If you're allowing arbitrary functions called from the user, you're in much bigger trouble. ;-) I originally noticed and reported the issue with the first example, which triggers a simple, common undefined index notice: <?php error_reporting(E_ALL); $array = array(); echo $array[$_GET['index']]; I only included the others for the sake of completeness. I've updated the code sample to note that.
- pilif 15y ago
 > Or someone can send you a URL that points to your dev server and contains a payload. That is theoretically possible, though what they would be able to do is to deal damage to my development install (if I don't to CSRF protection on POST operations) or they could potentially hijack my current session on my development system, which, too I don't deem too critical. > who uses shared hosting where display_errors is enabled by default init_set('display_errors', 0); Even on a shared host you can turn display_errors off. > There is no good reason PHP should be displaying this information unsanitized. I agree, especially because they already do some escaping of their own errors (and consequently don't escape while running in the CLI SAPI)

j_col 15y ago

I think if I was dumb enough to leave display_error on in production, I'd have a bigger problem with crackers being able to figure out sensitive details about my system from said error messages than worrying about them injecting scripts into them. Not saying that I have not seen this in the wild, but I have zero sympathy for those that don't carry out these basic steps to secure their production servers.

dave1010uk 15y ago

This is an example of CWE-81: "Improper Neutralization of Script in an Error Message Web Page" [1]. For those of you working with WordPress, this is the code you need in wp-config.php to log errors but not show them in web pages: define('WP_DEBUG', true); define('WP_DEBUG_DISPLAY', false); define('WP_DEBUG_LOG', true); [1] http://cwe.mitre.org/data/definitions/81.html http://cwe.mitre.org/data/definitions/81.html

deleted 15y ago

[deleted]

smcj 15y ago

Their bug tracker has a "Bogus" value? WTF? Well, PHP devs at its best: uninformed, ignorant, arrogant.

veloper 15y ago

This really falls under a few important coding mantras... Never trust the user. Never assume. Don't do stupid shit.

randomdrake 15y ago

I agree with many of the issues raised here and had my own issues with the blog post. I wrote up a response which contains more acceptable conclusions/fixes to the issues you present. http://randomdrake.com/2011/08/18/no-php-errors-and-notices-do-not-need-to-be-encoded/ http://randomdrake.com/2011/08/18/no-php-errors-and-notices-...

nbpoole 15y ago
Thanks, I just responded on your blog.