php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #81743 XSS via PDOException error
Submitted: 2022-12-12 15:41 UTC Modified: 2022-12-12 18:26 UTC
From: elbrinsomar666 at gmail dot com Assigned: cmb (profile)
Status: Not a bug Package: PDO Core
PHP Version: 8.1.13 OS: *
Private report: No CVE-ID: None
View Add Comment Developer Edit
 [2022-12-12 15:41 UTC] elbrinsomar666 at gmail dot com 
Description:
------------
There is an XSS through PDOException error as if the user input was like that

HTTP://localhost/vulnerable-script.php?q=/<script>alert(document.cookie)</script>

you will find that there is an error generated 

"Exception : SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '/<script>alert(document.cookie)</script>' at line 1"

which is the error message you will find that the user malicious input is reflected without any type of encoding (e.g. HTML entity encoding) which will lead from the error message to allowing a malicious user to execute javascript code to steal the victims cookie or execute malicious javascript code


Notes:
1- I know that this code is vulnerable to SQL injection but just assume that there is validation used by developers to prevent SQL Injection

2- I will use MySQL database as an example but I tested this vulnerability on SQLite too which obviously means that this vulnerability is on the PDO core and not related to a specific database

3- Tested on PHP version 8 and PHP version 5 but i didn't test it on versions between PHP8 and PHP5 yet 

Test script:
---------------
<?php
    $dsn = 'mysql:host=localhost;dbname=LoginSystem';
    $db = new PDO($dsn, 'username', 'password');
try {
    $q = $_GET['q'];
    $stmt = $db->query("SELECT * FROM users WHERE id=$q");
} catch (PDOException $e) {
print 'Exception : '.$e->getMessage();
}
?>

Expected result:
----------------
The above code is a common example of how developers use PDOException to trace errors or log them so the expected result is that the $e->getMessage() method should print PDOException as HTML entity encoded 

Impact: As I described if the input is reflected it will lead to Reflected XSS but a lot of times developers uses this method to log exception so imagine that this malicious XSS is executed at developers or an admin panel that will review this PDOException it will lead to critical impact if attacker stole developers or admins cookies with Blind XSS

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2022-12-12 17:02 UTC] cmb@php.net
-Status: Open +Status: Wont fix -Type: Security +Type: Bug -Assigned To: +Assigned To: cmb
 [2022-12-12 17:02 UTC] cmb@php.net 
This might require display_errors to be on, but the documentation
states[1]:

| This is a feature to support your development and should never
| be used on production systems (e.g. systems connected to the
| internet).

> that this malicious XSS is executed at developers or an admin
> panel that will review this PDOException

If a PHP log file is (partially) displayed in an admin panel, it
needs to be properly escaped, like every other output, by the PHP
userland developer.  The same escaping needs to be done, if an
exception message is echoed.

So no, this is not a security issue.  And especially the echoing
is nothing we can fix, because that very same code may run on the
command line, where applying HTML escaping would just be wrong.

[1] <https://www.php.net/manual/en/errorfunc.configuration.php#ini.display-errors>
 [2022-12-12 17:48 UTC] elbrinsomar666 at gmail dot com 
Hi cmb,

I wasn't asking to fix anything related to the echo function I mean
fixing the output of getMessage() by encoding it like that "Exception :
SQLSTATE[42000]: Syntax error or access violation: 1064 You have an
error in your SQL syntax; check the manual that corresponds to your
MariaDB server version for the right syntax to use near
&#039;/&lt;script&gt;alert(document.cookie)&lt;/script&gt;&#039; at line
1"

like any other function (e.g. simplexml_load_file()) if you entered 
wrong syntax for the previous function when it will display an error it
will display the content of entered input as HTML encoded in the error
message

so according to the same security design of functions, I was asking for
encoding the output from the getMessage() in the same way of secure
design for other functions

the main reason for asking that because the XSS vulnerability that I
described to you not list in the documentation and I searched on the
internet for finding anything related to this scenario but didn't find
anything that warns developers about the insecurity of output of
getMessage()



And you are right that every output should be escaped but if that output
comes from users but in getMessage() output the exception is expected to
come from PDO and not from user input that is why the PDOException is
trusted by developers and should the output of getMessage() method be
escaped

Thanks
 [2022-12-12 18:26 UTC] cmb@php.net
-Status: Wont fix +Status: Not a bug
 [2022-12-12 18:26 UTC] cmb@php.net 
> like any other function (e.g. simplexml_load_file()) if you entered 
> wrong syntax for the previous function when it will display an error it
> will display the content of entered input as HTML encoded in the error
> message

This happens due to html_errors[1] being enabled, but is not a
property of ::getMessage(), or happens generally when echoing. See
<https://3v4l.org/0tNfR>, for instance.  And running the given
reproduce script with html_errors enabled (and without catching
the exception), shows that the input would be escaped:

<b>Fatal error</b>:  Uncaught PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '=&lt;script&gt;alert(1)&lt;/script&gt;'

> the main reason for asking that because the XSS vulnerability that I
> described to you not list in the documentation and I searched on the
> internet for finding anything related to this scenario but didn't find
> anything that warns developers about the insecurity of output of
> getMessage()

There is nothing insecure about the return value of ::getMessage().
It is just a string, and you as userland developer need to properly
escape strings according to the context.  If you output JSON, you do
not want to HTML escape, and if we'd "fix" this, the JSON would be
broken.  Same for plain text output, etc.

[1] <https://www.php.net/manual/en/errorfunc.configuration.php#ini.html-errors>
 [2022-12-17 18:20 UTC] elbrinsomar666 at gmail dot com 
I understand the point that developers shouldn't allow errors on production and should use it in development only but allowing errors doesn't consider a vulnerability in itself

Anyway I made this 1-minute video as proof of the unsafe output of the getMessage() method that I described

I feel like you didn't get the scenario I'm trying to explain so I made a vulnerable lab for you to make sure that you get what I'm trying to explain
I recorded this 1-minute youtube video as a Proof of concept and that video should explain what I was mean about the output of the getMessage() method should be escaped like the simplexml_load_file() function

PoC youtube video: httPS://youtu.be/ekexMtdHQB4
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Tue Apr 23 12:01:31 2024 UTC