php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #77074 XSS through error messages
Submitted: 2018-10-27 19:08 UTC Modified: 2024-07-29 12:29 UTC
Votes:4
Avg. Score:3.5 ± 1.7
Reproduced:2 of 2 (100.0%)
Same Version:1 (50.0%)
Same OS:1 (50.0%)
From: david at grudl dot com Assigned: bukka (profile)
Status: Assigned Package: Output Control
PHP Version: 7.1 OS:
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: david at grudl dot com
New email:
PHP Version: OS:

 

 [2018-10-27 19:08 UTC] david at grudl dot com
Description:
------------
Displaying error messages is vulnerable to XSS, although the 'html_errors' is enabled.

Solution is to convert especial charactes < & in error message (ie to use  htmlspecialchars)

Test script:
---------------
<?php

echo ${'<script>alert(123);</script>'};

Expected result:
----------------
In web browser it should not pop up the alert window, but it should report:

"Notice: Undefined variable: <script>alert(123);</script> in test.php on line 3


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-10-27 19:15 UTC] requinix@php.net
-Summary: david@grudl.com +Summary: XSS through error messages
 [2018-10-27 19:20 UTC] nikic@php.net
We're definitely applying htmlspecialchars to errors in html_errors mode. Can you please double check that it is really enabled (e.g. by printing the html_errors ini setting in the same script you are testing the error message)?
 [2018-10-27 19:30 UTC] requinix@php.net
The escaping only happens for E_ERROR and E_PARSE.
https://github.com/php/php-src/blob/php-7.3.0RC4/main/main.c#L1336
 [2018-10-27 19:31 UTC] david at grudl dot com
-Summary: XSS through error messages +Summary: david@grudl.com -PHP Version: 7.3.0RC4 +PHP Version: 5.6.5
 [2018-10-27 19:31 UTC] david at grudl dot com
Yes, html_errors is enabled.

(Sorry for wrong description, autofilling in browser surprised me ;-)
 [2018-10-27 19:35 UTC] requinix@php.net
-Summary: david@grudl.com +Summary: XSS through error messages -PHP Version: 5.6.5 +PHP Version: 7.1
 [2018-10-27 19:56 UTC] spam2 at rhsoft dot net
> The escaping only happens for E_ERROR and E_PARSE

why?

also in cli-mode escape sequences should be filtered, we implemented a lot of sanitze stuff because we don't trust PHP in that context but different behavior depending on the error type is a no-go and explains some headache from the past
 [2018-10-27 20:02 UTC] requinix@php.net
> why?
I don't know. I'm not giving an explanation, just stating the current behavior.

The change to give E_ERROR (E_PARSE was added later) special treatment dates back to 5.1.2.
https://github.com/php/php-src/commit/2796160d15463a04acd7088fea379593d2c9caa0
 [2018-10-27 20:12 UTC] nikic@php.net
@requinix: The code I was looking at is https://github.com/php/php-src/blob/php-7.3.0RC4/main/main.c#L945, which always escapes -- but now I realize that php_verror is only going to be used for docref type errors, not for "raw" errors.

Possibly this is also why it special cases ERROR and PARSE, to avoid double escaping.
 [2018-10-27 21:21 UTC] spam2 at rhsoft dot net
> Possibly this is also why it special cases ERROR and PARSE, to avoid double escaping

wouldn't it be smarter to act like http://php.net/manual/de/function.htmlentities.php  and have bool $double_encode=TRUE in that case using FALSE instead special handeling for the sake of consistency?
 [2018-10-27 21:43 UTC] requinix@php.net
It should always encode because then that will always show the original value. If OP's already bizarre situation was even worse, like
  echo ${'&lt;script&gt;alert(123);&lt;/script&gt;'};
then I should see those "lt"s and "gt"s in the error message ("&amp;lt/gt;" in the output) because that's what the variable was actually named.
 [2018-10-27 21:47 UTC] nikic@php.net
An additional issue here is that docref also inserts markup for the documentation links, and we of course wouldn't want to escape that part.
 [2018-10-27 22:21 UTC] spam2 at rhsoft dot net
> An additional issue here is that docref also 
> inserts markup for the documentation links,
> and we of course wouldn't want to escape that part.

is a cry for refactoring - do you tell me you can't distinct between that and escaping happens at the of everything independent from where it came and what it was?
 [2024-07-29 12:29 UTC] bukka@php.net
-Assigned To: +Assigned To: bukka
 [2024-07-29 12:29 UTC] bukka@php.net
So I have done some research and there is an escaping for all docref errors (e.g. usually errors from extensions / functions). This triggers php_verror that creates the message (escaped) and pass it to zend_error_zstr which calls zend_error_zstr_at. However the various compiler and other errors go through zend_error which calls zend_error_va_list and that calls zend_error_zstr_at. This path does not get escaped. It eventually calls zend_error_cb which is by default set to error_function of zend_utility_functions struct which is be default php_error_cb. No other core extension sets it but it is available for external extension so changing anything in it is essentially ABI break.

So I can think of a couple of solutions and none of them is really ideal.

1. Extend all needed definitions and add some flags param that could indicate if escaping has been done. The flag would be then check in php_error_cb instead of that error check. This is obviously an ABI break. We could potentially have _ex functions so the old one would be still available but we would still need to extend utility function so still ABI break.

2. Overload type param as it's an int and there's a space. This would require some masking and unmasking when testing of the type. It would be also problematic if any external extension overload zend_error_cb and quite hard to catch issue. It seems a bit messy as well.

3. Escape the zend_error path too somehow and removing error escaping from php_error_cb. This might require a new utility function as escaping is not available in Zend and we need to somehow bridge it to main...
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 11:01:29 2024 UTC