jbowens / jBBCode

A lightweight but extensible BBCode parser
http://jbbcode.com
MIT License
164 stars 32 forks source link

Documentation suggestion #2

Closed CriseX closed 11 years ago

CriseX commented 11 years ago

Just a suggestion about the examples on the web page, you should at least add the following notes to them as general recommendations.

  1. Before calling parse(..) it might be a good idea to use something like htmlspecialchars($text, ENT_COMPAT, 'UTF-8');
  2. Consistently using double quotes in your html replacements when adding bbcodes (in conjunction with previous) will provide at least rudimentary XSS protection.

I know security can be seen as somewhat non-issue for a middleware like jbbcode, however, ideally the concept of {option} and {param} could be enhanced/expanded to allow things like {attribute} etc. with some limits, or at the very least allowing users to externally specify the default CodeDefinition to use for bbcode's added via addBBCode.

Yes it is possible of course by deriving Parser and re-implementing addBBCode, but a geniune interface for it would be nicer.

al63 commented 11 years ago

same

CriseX commented 11 years ago

After giving it some thought, while string replacements are nice, why not leave the method of replacement up to the user... ie. rather than supply html replacement to the addBBCode use a php callable, which means whoever adds the bbcode is now able to do whatever sanity checks they need.

It would also allow addBBCode to theoretically support tags with multiple attributes, because said attributes could be passed to the provided callable as an associative array rather than having to map them to some identifier scheme (of course the current parser class would need to be adapted for it).

brandonfredericksen commented 11 years ago

Can you elaborate on the double quotes when adding new bbcode?

$parser->addBBCode("quote", 'div class="quote"{param}/div');

is the above secure? (took out < and >)

CriseX commented 11 years ago

To be clear about what I said in the first post, following those instructions is not 100% secure, as for your quote question, yes, although it only matters if you allow user input within attributes so in above example it would actually make no difference.

Regarding security, consider the following... even with my suggestion the following BBCode is exploitable: $parser->addBBCode("url", "<a href=\"{option}\">{param}</a>", true); because even with the escaping applied before calling parse and even with double quotes around attribute name, preventing adding of additional attributes, the value itself (option) is not validated to be an actual url... so user could embed say javascript in it.

jbowens commented 11 years ago

I think your concerns may have at least partially been addressed by 426e5248.