Home » HTML » HTML_BBCodeParser » Bug #4844

[2005-07-16 12:05 UTC] plyrvt at mail dot ru

Description:
------------
Many Filters allow arbitrary HTML injection via attributes.
This makes no sense in using HTML_BBCode_Parser at all.

Reproduce code:
---------------
[align=foo'>alert('JavaScript_Enabled');][/align]

Expected result:
----------------

Actual result:
--------------
alert('JavaScript_Enabled');'>

[2005-07-20 18:55 UTC] plyrvt at mail dot ru

Suggested bugfix:

After 548 line in BBCodeParser.php add:

foreach ($tag['attributes'] as $a => $v) {
$v=htmlspecialchars($v);
....
}

Another optimization suggestion:
$this->_filters[$filter] = new $class;
assign filter class by reference (filter class is quite huge to copy because it extends base class)

$this->_filters[$filter] = & new $class;

[2005-07-20 19:17 UTC] plyrvt at mail dot ru

--- BBCodeParser.orig Wed Jul 20 22:18:18 2005
+++ BBCodeParser.php Wed Jul 20 22:17:23 2005
@@ -217,7 +217,7 @@
if (!class_exists($class)) {
PEAR::raiseError("Failed to load filter $filter", null, PEAR_ERROR_DIE);
}
- $this->_filters[$filter] = new $class;
+ $this->_filters[$filter] = & new $class;
$this->_definedTags = array_merge($this->_definedTags, $this->_filters[$filter]->_definedTags);
}
}
@@ -546,6 +546,7 @@
if ($this->_options['quotestyle'] == 'single') $q = "'";
if ($this->_options['quotestyle'] == 'double') $q = '"';
foreach ($tag['attributes'] as $a => $v) {
+ $v=htmlspecialchars($v);
if ( ($this->_options['quotewhat'] == 'nothing') ||
($this->_options['quotewhat'] == 'strings') && (is_numeric($v)) ) {
$this->_parsed .= ' '.sprintf($this->_definedTags[$tag['tag']]['attributes'][$a], $v, '');

[2005-10-20 22:12 UTC] seth at pricepages dot org

I've fixed this bug and a number of others. Please take a
look if you can, because I'd expect there to be new bugs in
a rewrite of this size. (I'd call it version 2.0-beta)

Link:
http://pricepages.org/bbcode/BBCodeParser.zip

* Unit tests!

* More currently open bugs that have been fixed:
5609
4844
3447
1979
373
2580
3775

* I rewrote the _buildTagArray() function. It had been using
half of the execution time in my tests. A TODO: in the
comments said "rewrite this function". It is now almost 2x
faster than the original function in my informal tests.

* I rewrote _validateTagArray() in an effort to make it more
useful and faster. Mission accomplished. It would be easier
to read, too, but there is quite
a bit more going on there. But commenting lines probably
equal code lines.

* Output should _always_ be XHTML 1.1 compatible.

* Per discussion on the Developer list, you can now pass a
flag that determines the action on an error or warning
during parsing. Actions include: correcting the problem,
aborting parsing, ignoring the invalid tag, and deleting the
invalid tag.

Examples:
If you only accept valid input, set both warn and error to
'abort' and parsing will be aborted as soon as a problem is
found.
If you accept invalid feedback, but want to give the user
feedback which tags caused problems, then set both error and
warn to 'ignore' and the bad tags will be displayed.
If you want to make the output as pretty as possible, then
you want to auto correct when you can and delete when you
can't. Set the options to delete on error, and correct on
warn.
(There are twelve combinations to fit various situations.)

* I've attempted to maximize BC, but here are the only ways
in which BC is broken (to my knowledge):

Custom list numbering is now ignored for XHTML 1.1 reasons.
I reverted back to single quotes by default. HTML is now
automatically escaped to fix a few "bugs" (I believe that
Text_Wiki also automatically escapes HTML). Old filters are
incompatible with the new filters (the 'allowed' tag
variable has been replaced with a more powerful set of
variables and they use a different format).

~Seth

[2005-12-20 09:33 UTC] anders at norrbring dot se

Seems like the ini file in the example doesn't conform to the expected format.. I haven't been digging into it yet, but I get lots of errors trying to use it..

Errors that indicates "no delimiter" in strstr on line 194 in BBCodeParser.php