search

apostrophecms / sanitize-html

Clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis. Built on htmlparser2 for speed and tolerance
MIT License
3.69k stars 351 forks source link

iframe src attribute not allowed in some browsers when iframe options set #504

Closed mattclough1 closed 2 years ago

mattclough1 commented 2 years ago

To Reproduce

Step by step instructions to reproduce the behavior:

  1. Open a console in Chrome or Firefox
  2. Paste new URL('//cdn.iframe.ly', 'relative://relative-site/1/')
  3. Inspect the returned object's values
  4. Note hostname is an empty string
  5. Repeat steps 1–3 in Safari
  6. Note that hostname is now cdn.iframe.ly

Expected behavior

sanitize-html, when either allowedIframeHostnames or allowedIframeDomains option is set, should allow a url such as //cdn.iframe.ly as the src attribute for an iframe

Describe the bug

When either allowedIframeHostnames or allowedIframeDomains option is set, sanitize-html attempts to set the variable allowed to a truthy value by using the parsed URL's hostname or domain, then deleting the src attribute if allowed is not a truthy value. In Chrome and Firefox, some URLs may be disallowed if these options are set due to hostname being an empty string.

Details

Additional context: Tested Browsers: Chrome 94.0.4606.81 Safari 15.0 (16612.1.29.41.4, 16612) Firefox 93.0

boutell commented 2 years ago

Interesting. Appears to be a limitation of the URL class implementation in these browsers. Not an issue in Node.js, which is the primary use case, but browser use is supported, so a PR would be welcome. Perhaps it's possible to do a PR for this that doesn't involve dragging in an entire re-implementation of URL parsing.

On Thu, Oct 14, 2021 at 4:06 PM Matt Clough @.***> wrote:

To Reproduce

Step by step instructions to reproduce the behavior:

  1. Open a console in Chrome or Firefox
  2. Paste new URL('//cdn.iframe.ly', 'relative://relative-site/1/')
  3. Inspect the returned object's keys
  4. Note hostname is an empty string
  5. Repeat steps 1–3 in Safari
  6. Note that hostname is now cdn.iframe.ly

Expected behavior

sanitize-html, when either allowedIframeHostnames or allowedIframeDomains option is set, should allow a url such as //cdn.iframe.ly as the src attribute for an iframe Describe the bug

When either allowedIframeHostnames or allowedIframeDomains option is set, sanitize-html attempts to set the variable allowed to a truthy value by using the parsed URL's hostname or domain, then deleting the src attribute if allowed is not a truthy value. In Chrome and Firefox, some URLs may be disallowed if these options are set due to hostname being an empty string. Details

Additional context: Tested Browsers: Chrome 94.0.4606.81 Safari 15.0 (16612.1.29.41.4, 16612) Firefox 93.0

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/504, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27JCD7EWW3CVG7V3GFDUG4Z5DANCNFSM5GAOSAYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his