4 ms·

`value.startsWith('javascript:')` and there you have a vulnerability. There can be arbitrary white-space before the URL, so you'd need to do `value.trim().start

by panzi 3y ago

`value.startsWith('javascript:')` and there you have a vulnerability. There can be arbitrary white-space before the URL, so you'd need to do `value.trim().startsWith('javascript:')` instead. However, I much prefer white listing instead, i.e. only allowing `http:`, `https:` and maybe `mailto:`, `ftp:`, `sftp:` and such. Maybe allow relative URLs starting with `/`. Maybe. Though that would mean to correctly handle all attributes that actually can be URLs. Again, I'd just white list a few tags plus a few of their attributes.

simonw 3y ago
You mean from this article "Sanitize HTML strings"? https://phuoc.ng/collection/html-dom/sanitize-html-strings/ https://phuoc.ng/collection/html-dom/sanitize-html-strings/ Yeah, that article really shouldn't imply that sanitization is "that easy". It does at least mention https://github.com/cure53/DOMPurify https://github.com/cure53/DOMPurify at the end but it should LOUDLY argue against attempting to write this particular thing yourself and promote that exclusively in my opinion. Filed an issue about this here: https://github.com/phuocng/html-dom/issues/281 https://github.com/phuocng/html-dom/issues/281
- runarberg 3y ago
  There is also a future way of doing this with vanilla JavaScript using the Sanitize API. It is enabled under flags in most browsers currently. https://developer.mozilla.org/en-US/docs/Web/API/HTML_Sanitizer_API https://developer.mozilla.org/en-US/docs/Web/API/HTML_Saniti...
  - panzi 3y ago
    Yes, that one seems promising, but I really would like more control on allowed URLs. I.e. a flag to allow/disallow cross domain URLs anywhere (<a href>, <link href>, <form action>, <img src>, <img srcset>, <iframe src> etc. etc.) And does it strip any styles (style attribute, style tag, link stylesheet)? If yes, can I allow certain styles somehow? Needs work IMO.
jfhr 3y ago
Unless you need to support ancient browsers [1], I'd go the easy way and leave the URL parsing to the browser, e.g. `['http:', 'https:', 'mailto:', 'tel:'].includes(new URL(value, location.origin).protocol)` [1] https://caniuse.com/url https://caniuse.com/url
- keepamovin 3y ago
  JavaScript elegance is always welcome!

by panzi 3y ago

simonw 3y ago

You mean from this article "Sanitize HTML strings"? https://phuoc.ng/collection/html-dom/sanitize-html-strings/ https://phuoc.ng/collection/html-dom/sanitize-html-strings/ Yeah, that article really shouldn't imply that sanitization is "that easy". It does at least mention https://github.com/cure53/DOMPurify https://github.com/cure53/DOMPurify at the end but it should LOUDLY argue against attempting to write this particular thing yourself and promote that exclusively in my opinion. Filed an issue about this here: https://github.com/phuocng/html-dom/issues/281 https://github.com/phuocng/html-dom/issues/281

runarberg 3y ago
There is also a future way of doing this with vanilla JavaScript using the Sanitize API. It is enabled under flags in most browsers currently. https://developer.mozilla.org/en-US/docs/Web/API/HTML_Sanitizer_API https://developer.mozilla.org/en-US/docs/Web/API/HTML_Saniti...
- panzi 3y ago
  Yes, that one seems promising, but I really would like more control on allowed URLs. I.e. a flag to allow/disallow cross domain URLs anywhere (<a href>, <link href>, <form action>, <img src>, <img srcset>, <iframe src> etc. etc.) And does it strip any styles (style attribute, style tag, link stylesheet)? If yes, can I allow certain styles somehow? Needs work IMO.

jfhr 3y ago

Unless you need to support ancient browsers [1], I'd go the easy way and leave the URL parsing to the browser, e.g. `['http:', 'https:', 'mailto:', 'tel:'].includes(new URL(value, location.origin).protocol)` [1] https://caniuse.com/url https://caniuse.com/url

keepamovin 3y ago
JavaScript elegance is always welcome!