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

Regular expressions still aren’t the right way #4

Closed
charmander opened this issue Sep 17, 2013 · 6 comments
Closed

Regular expressions still aren’t the right way #4

charmander opened this issue Sep 17, 2013 · 6 comments

Comments

@charmander
Copy link

I’d appreciate it if you reopened the old issue — it’s more likely that someone notices that way — but here’s another “stripping creates exploit” problem in stripUnsafeAttrs instead of stripUnsafeTags:

<a href= href="javascript:""javascript:anything">Click me!</a>
@charmander
Copy link
Author

Interesting! stripComments also still doesn’t strip comments.

@danmactough
Copy link
Owner

Closed by b35dea0 as XSS-related filtering is delegated to chriso/node-validator.

stripComments also still doesn’t strip comments.

Sure does. If there's something missing, a proper bug report or pull request is welcome.

@charmander
Copy link
Author

sanitize.stripComments("<!-- This HTML is <em>commented out</em>. -->")

@charmander
Copy link
Author

Congratulations on bringing in another sanitizer, but as this was directed RSS feeds, it’ll be unfortunate if someone ever wants to write about cookies.

On the other hand, security that defeats itself is my favourite kind. There’s still a vulnerability, but I think I have to go report that to node-validator. (I could also not, because regular expressions aren’t the right way to go about this.)

@charmander
Copy link
Author

(For reference, it’s validatorjs/validator.js#223.)

@charmander
Copy link
Author

node-validator has removed its “XSS sanitizer” because it didn’t work; you should try going with Caja, which almost certainly will. It’ll be faster to boot.

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