Bug #81743 [Com]: XSS via PDOException error

Edit report at https://bugs.php.net/bug.php?id=81743&edit=1 ID: 81743 Comment by: elbrinsomar666 at gmail dot com Reported by: elbrinsomar666 at gmail dot com Summary: XSS via PDOException error Status: Not a bug Type: Bug Package: PDO Core Operating System: * PHP Version: 8.1.13 Assigned To: cmb Block user comment: N Private report: N New Comment: 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 Previous Comments: ------------------------------------------------------------------------ [2022-12-12 18:26:46] 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-12 17:48:29] 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 17:02:00] 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 15:41:06] 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 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=81743&edit=1