Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change CONTENT_TAGS to blacklist #65

Closed
neilj opened this issue May 29, 2015 · 1 comment
Closed

Change CONTENT_TAGS to blacklist #65

neilj opened this issue May 29, 2015 · 1 comment

Comments

@neilj
Copy link
Contributor

neilj commented May 29, 2015

I wanted to check this before submitting a pull request: at the moment, if you use KEEP_CONTENT, the tag has to match a whitelist for its contents to be kept. This is not necessary for security, since the contents is still sanitised just like any other content in the page, and is annoying if you are sanitising random HTML from sources which may use arbitrary non-standard tags to wrap bits of content (resulting in chunks being stripped by the sanitiser).

I suggest changing the CONTENT_TAGS whitelist to a FORBID_CONTENTS blacklist; the blacklist is not needed for security, but we do want to remove the contents of script tags and perhaps a few others by default, since we know they are not designed to have user-visible content.

Anyone have a problem with this/see a security flaw I've missed? If not, I'll submit a pull request.

@cure53
Copy link
Owner

cure53 commented May 29, 2015

I don't see a problem in terms of security. It might however break existing implementations so we need to document the change carefully with the next release.

@cure53 cure53 closed this as completed Jun 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants