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

DOMPurify support. #102

Open
cscott opened this issue Sep 9, 2017 · 8 comments
Open

DOMPurify support. #102

cscott opened this issue Sep 9, 2017 · 8 comments

Comments

@cscott
Copy link
Collaborator

cscott commented Sep 9, 2017

https://www.npmjs.com/package/dompurify should work under domino.
It appears that outerHTML#set is one of the things needed.
See: https://gerrit.wikimedia.org/r/363156

@daniel-nagy
Copy link

@cscott is setting the outerHTML property not supported? My server rendered Angular bundles are not working when I try to bind to the outerHTML property.

<svg [outerHTML]="mySvgIcon"></svg>

@cscott
Copy link
Collaborator Author

cscott commented Jul 12, 2018

The setters for Document#outerHTML, Document#innerHTML, DocumentFragment#outerHTML, DocumentFragment#innerHTML, Element#outerHTML, and Element#innerHTML are all util.nyi (Not Yet Implemented) right now.

Probably not too hard to get that working, just a bit of tedious reading of the parsing spec to ensure contexts and templates are correct. HTMLElement#innerHTML has a working setter in htmlelts.js, so the other implementations should look very similar to that.

cscott added a commit to cscott/domino that referenced this issue Aug 13, 2018
These definitions are needed to ensure proper sanitization by
DOMPurify (fgnass#102).
cscott added a commit to cscott/domino that referenced this issue Aug 13, 2018
The "insert a foreign element" operation in the HTML parsing algorithm
differs from the standard `Document#createElementNS()` operation: it
explicitly sets the prefix to `null`, and creates a `localName` which
may contain `:` characters.  Duplicate this behavior so that prefixes
aren't lost when an SVG element is inserted, since the HTML serialization
algorithm otherwise suppresses prefixes on SVG elements.

This is needed for DOMPurify (fgnass#102).
cscott added a commit to cscott/domino that referenced this issue Aug 13, 2018
These definitions are needed to ensure proper sanitization by
DOMPurify (fgnass#102).
cscott added a commit to cscott/domino that referenced this issue Aug 13, 2018
The "insert a foreign element" operation in the HTML parsing algorithm
differs from the standard `Document#createElementNS()` operation: it
explicitly sets the prefix to `null`, and creates a `localName` which
may contain `:` characters.  Duplicate this behavior so that prefixes
aren't lost when an SVG element is inserted, since the HTML serialization
algorithm otherwise suppresses prefixes on SVG elements.

This is needed for DOMPurify (fgnass#102).
@cscott
Copy link
Collaborator Author

cscott commented Aug 14, 2018

domino 2.1.0 works with DOMPurify. I just need to submit my patch upstream to DOMPurify.

@thesocialdev
Copy link

@cscott did it move forward? I'm trying to implement a sanitizer for the PCS service (mobile-html endpoint). The first choice I had was the sanitize-html package, but its minified size is prohibitive for our bundled size limit. DOMPurify is lightweight, but I'm worried that it's not reliable with domino, although it works in my code.

@cscott
Copy link
Collaborator Author

cscott commented Mar 20, 2020

I'd like to talk some more about the desired use case and spec. MediaWiki has a very specific (and bespoke) sanitizer, and new attributes are added/removed from time to time (we just added one this week, for instance). Adding new incompatible sanitization mechanisms doesn't seem like necessarily the right way to proceed.

@cscott
Copy link
Collaborator Author

cscott commented Mar 20, 2020

The necessary DOMPurify patches to support domino are at https://github.com/cscott/DOMPurify/tree/domino-support

@thesocialdev
Copy link

The specific use case is to sanitize the input from displaytitle and enforce allowed tags when inserting into the lead section using innerHtml [1].

TBH, I'm not sure if that's really necessary since I think it's a safeish content since it's coming from the mwapi. In that sense, I lack knowledge regarding mwapi/displaytitle sanitization.

[1] https://github.com/wikimedia/mobileapps/blob/master/pagelib/src/transform/EditTransform.js#L120

@giniedp
Copy link

giniedp commented Mar 20, 2023

I'm stuck on this. Any progress in supporting DOMPurify?
Angular: 15
DOMPurify: 3.0.1
domino: 2.1.6

Client Side code works well, but server side (with domino) is sanitized to an empty string.

From what i can track down, it is the getElementsByTagName that does not return anything
https://github.com/cure53/DOMPurify/blob/bcb0f7f0a1b9ba902d060cb44985fc7f5a640a4b/src/purify.js#L864
https://github.com/fgnass/domino/blob/master/lib/Element.js#L270

DOMPurify sets the content via #innerHTML.
https://github.com/cure53/DOMPurify/blob/bcb0f7f0a1b9ba902d060cb44985fc7f5a640a4b/src/purify.js#L845

yet the document readyState is always loading.

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

4 participants