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

Should we protect against DOM clobbering by default? #14

Closed
cure53 opened this issue Mar 8, 2014 · 3 comments
Closed

Should we protect against DOM clobbering by default? #14

cure53 opened this issue Mar 8, 2014 · 3 comments

Comments

@cure53
Copy link
Owner

cure53 commented Mar 8, 2014

Right now, DOMPurify protects itself against DOM Clobbering. But the resulting markup itself still has clobbering potential when being used by the sanitizing website.

Should be by default prevent that? My thought would be: If an element's name is at the same time a property in document, the element should be removed. What do you think? We would prevent XSS and DOM clobbering.

//cc @fhemberger @mathiasbynens

@mathiasbynens
Copy link
Contributor

Sounds good to me! The only downside is that sanitize() wouldn’t be a pure/deterministic function anymore… But that doesn’t really matter. Improved security is more important.

@cure53
Copy link
Owner Author

cure53 commented Mar 8, 2014

We can always use a flag. Question is more like: default or not ;)

cure53 pushed a commit that referenced this issue Mar 8, 2014
# added DOM clobbering protection for output
# added new config flag SANTIZE_DOM, default true
# updated README with new config options
cure53 pushed a commit that referenced this issue Mar 8, 2014
# added clobbering tests to expect.json
@cure53
Copy link
Owner Author

cure53 commented Mar 19, 2014

I think this can be close, it's part of the code, behind a config flag and no one objected so far ;)

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

No branches or pull requests

2 participants