[Phpmyadmin-devel] htmlspecialchars in PMA_Error

The htmlspecialchars escaping of error messages in PMA_Error, introduced in https://github.com/phpmyadmin/phpmyadmin/commit/656809ac3bdc8ba61b84657b8789..., causes problems with error messages containing links. See this photo for reference: http://cl.ly/FQ0H, read main.php, lines 293 and 329. Do we prefer using trigger_error() or the direct PMA_Message::display variant? -- mynetx

Hi Dne Thu, 29 Mar 2012 22:17:42 +0200 "J.M." <me@mynetx.net> napsal(a):
The htmlspecialchars escaping of error messages in PMA_Error, introduced in https://github.com/phpmyadmin/phpmyadmin/commit/656809ac3bdc8ba61b84657b8789..., causes problems with error messages containing links. See this photo for reference: http://cl.ly/FQ0H, read main.php, lines 293 and 329.
Do we prefer using trigger_error() or the direct PMA_Message::display variant?
Generally anything what comes as an error from PHP needs to be escaped, so there are two options: - pass our error messages to trigger_error as some object (let's call it SafeString for now) and if error handler sees SafeString, it won't do any processing of that - do not use trigger_error for anything what includes markup I'd prefer first solution (actually marking strings as safe to output is generally useful thing to prevent XSS). -- Michal Čihař | http://cihar.com | http://blog.cihar.com

Do we prefer using trigger_error() or the direct PMA_Message::display variant?
Generally anything what comes as an error from PHP needs to be escaped, so there are two options:
- pass our error messages to trigger_error as some object (let's call it SafeString for now) and if error handler sees SafeString, it won't do any processing of that
- do not use trigger_error for anything what includes markup
I'd prefer first solution (actually marking strings as safe to output is generally useful thing to prevent XSS).
The concept would be great—but trigger_error only accepts strings. See the signature here: bool trigger_error ( string $error_msg [, int $error_type = E_USER_NOTICE ] ) I suggest a workaround. We might store the message’s MD5 hash in a global variable, like this: function whitelist_error($errstr) { if (!isset($GLOBALS['error_whitelist'])) { $GLOBALS['error_whitelist'] = array(); } $GLOBALS['error_whitelist'][] = md5($the_message); } Within the PMA_Error_Handler::handleError function, we would check if the current $errstr is whitelisted: // if error message is not whitelisted, sanitize it if (! isset($GLOBALS['error_whitelist']) || ! in_array(md5($errstr), $GLOBALS['error_whitelist'])) { $errstr = htmlspecialchars($errstr); } $error = new PMA_Error($errno, $errstr, $errfile, $errline); But looking at this, do we still prefer trigger_error over directly echoing PMA_Error objects as with The phpMyAdmin configuration storage is not completely configured in main.php line 322?
participants (2)
-
J.M.
-
Michal Čihař