Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 1.2.4

Bug #5609 BBCodeParser allows XSS
Submitted: 2005-10-05 10:33 UTC Modified: 2007-06-03 13:18 UTC
From: vanderaj at greebo dot net Assigned: dufuz
Status: Closed Package: HTML_BBCodeParser
PHP Version: 5.0.4 OS: MacOS X
Roadmaps: (Not assigned)    

 [2005-10-05 10:33 UTC] vanderaj at greebo dot net
Description: ------------ HTML_BBCodeParser fails to validate URL tags properly, and allows XSS through. Test script: --------------- This post here describes the identical issue in phpBB 2.0.14 and below. It affects HTML_BBCodeParser in *exactly* the same way - all the examples in this exploit work as described. Expected result: ---------------- XSS in any of the supported attributes should not work. Actual result: -------------- XSS occurs. Try it and see.


 [2005-10-05 12:52 UTC] toggg
As a side note, I tested out these XSS with the proposed replacement engined by Text_Wiki_BBCode and they don't work. Only a few schemes (configurable) are accepted and *not* those listed in the link above.
 [2005-10-18 08:09 UTC] quipo
Thank you for this bug report. To properly diagnose the problem, we need a backtrace to see what is happening behind the scenes. To find out how to generate a backtrace, please read Once you have generated a backtrace, please submit it to this bug report and change the status back to "Open". Thank you for helping us make PEAR better. I've applied the suggested patch. Can I close this bug report?
 [2006-12-29 05:51 UTC] andrei_nikolov at mail dot bg (Andrei Nikolov)
suggestion for fixing the bug: adding new variable in the class: /** * Counter of closing [url] tags which must be left unparsed (XSS prevention) * * @access private * @var int */ var $skipClosingTagsURL = 0; ........... in function _buildParsedString(): // opening tag case 1: //Prevents XSS attacks: if (($tag['tag'] == 'url' || $tag['tag'] == 'img') && preg_match('#(script|about|applet|activex|chrome):#is', $tag['text'])) { $this->_parsed .= $tag['text']; if ($tag['tag'] == 'url') $this->skipClosingTagsURL++; break; } ......... // closing tag case 2: if ($this->_definedTags[$tag['tag']]['htmlclose'] != '') { if ($tag['tag'] == 'url' && $this->skipClosingTagsURL) { $this->_parsed .= $tag['text']; $this->skipClosingTagsURL--; } else $this->_parsed .= '</'.$this->_definedTags[$tag['tag']]['htmlclose'].'>'; } break; ....... function parse() { $this->skipClosingTagsURL = 0; $this->_preparse(); $this->_buildTagArray(); $this->_validateTagArray(); $this->_buildParsedString(); }
 [2007-06-03 13:18 UTC] dufuz (Helgi ├×ormar)
This bug has been fixed in CVS. If this was a documentation problem, the fix will appear on by the end of next Sunday (CET). If this was a problem with the website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better.
 [2007-07-13 17:07 UTC] frankysanders (Lance Sanders)
Tags such as the following are also an issue. I'm working on a solution. [url=javascript:alert("Pwned By DSK");]click here[/url]
 [2007-07-13 19:12 UTC] frankysanders (Lance Sanders)
Proposed solution: Replace $v = preg_replace('#(script|about|applet|activex|chrome):#is', "\\1:",$v); With $v = preg_replace('#(script|about|applet|activex|chrome):#is', "\\1:", html_entity_decode($v));
 [2007-07-13 19:50 UTC] frankysanders (Lance Sanders)
My proposed fix fails in some cases. [img]&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28[/img] See for details
 [2007-07-13 21:00 UTC] frankysanders (Lance Sanders)
See _buildParsedString This is a proposed fix. Not elegant but get's the job done for my purposes. Essentially users will not be able to post tags like &# in images or links but I am comfortable living with this compromise foreach ($tag['attributes'] as $a => $v) { if (($tag['tag'] == 'url' || $tag['tag'] == 'img')){ $v = (strpos($v,"&#") === false) ? $v : ""; }