braintree / sanitize-url

MIT License
312 stars 35 forks source link

Non-valid URL passes #5

Closed luwes closed 6 years ago

luwes commented 6 years ago

http://aa.com/</script>"><img src=x onerror='prompt(document.domain)'>

This value as URL seems to pass.

crookedneighbor commented 6 years ago

Maybe I'm misunderstanding the attack vector here, but this lib is intended just for sanitizing urls, not html.

IE, if you were to call

var sanitizeUrl = require('@braintree/sanitize-url').sanitizeUrl;

// doesn't add an img tag to the page, so the js doesn't activate
document.querySelector('a#my-link').href = sanitizeUrl(`http://example.com/</script>"><img src=x onerror='prompt(document.domain)'>`);

// doesn't add an img tag to the page, so the js doesn't activate
window.location = sanitizeUrl(`http://example.com/</script>"><img src=x onerror='prompt(document.domain)'>`);

If you did:

var sanitizeUrl = require('@braintree/sanitize-url').sanitizeUrl;

document.querySelector('#my-div').innerHTML = sanitizeUrl(`http://example.com/</script>"><img src=x onerror='prompt(document.domain)'>`);

It would inject the html, but that's not really a concern of the library. All the library cares about is that the url itself is sanitized before used for navigation.

Can you share a full example where there is a vulnerability?

luwes commented 6 years ago

Thanks for the quick answer @crookedneighbor! I misunderstood, thanks for clearing that up.

In our case we used urls returned from our back-end in a JSON response, images and links to populate in our simple JS template. Similar too mustache. We escape front-end only (https://stackoverflow.com/a/32848351/268820)

I found out that a simple encodeURIComponent() after sanitizing was enough to prevent this XSS issue.