NodeBB-Community / nodebb-plugin-composer-redactor

Redactor Composer for NodeBB
GNU General Public License v3.0
38 stars 28 forks source link

Sanitize More, fix #13 #14

Closed yariplus closed 9 years ago

yariplus commented 9 years ago

Fixes things:

Removed wildcard attributes, replaced with list of 'safe' attributes plus 'style'. Added library to parse unsafe 'style' attributes. (Couldn't find a good solution for this.) Added post edit sanitizing. You were missing the 'hr' and 'source' tags.

drewdotpro commented 9 years ago

@yariplus thank you for the pull request. Can I enquire where you got the list of safe attributes? I just want to ensure that there are no attributes added by Radactor's WYSIWYG that are then stripped.

yariplus commented 9 years ago

I got it from here https://validator.w3.org/feed/docs/warning/SecurityRiskAttr.html

I haven't found any problem using it so far. It does seem a little too restrictive though, maybe this is better: https://pythonhosted.org/feedparser/html-sanitization.html

Even that one seems to be missing some html5 attributes. I'll have to do a more thorough analysis.

yariplus commented 9 years ago

I prepared a better organized attribute list that only lists the valid and 'safe' attributes for each tag... and I noticed the library I was using to sanitize css is stripping out a few required attributes.

I think I need a different solution for sanitizing the style attribute.

yariplus commented 9 years ago

@drewdotpro This should be good now. It fixes most of the problems. And all of the valid attributes for the previous tag list are entered, so it shouldn't strip out anything needed.