search

braintree / sanitize-url

MIT License
307 stars 35 forks source link

Invalid URL not being properly sanitized #65

Closed rafaelguedes closed 7 months ago

rafaelguedes commented 7 months ago

Firstly, thank you for your efforts on this valuable open-source initiative.

I've observed that some common bypasses for URL sanitization seem to be getting through, particularly examples like:

sanitizeUrl("javajavascript:alert(document.domain)")

Is this intentional, or is it something we should work on improving?

hollabaq86 commented 7 months ago

👋 thanks for reaching out! Honestly, with URL sanitization it feels like a game of whack-a-mole sometimes 😄 or I could see a scenario where we made assumptions that a function made public in this library is being used internally by us where we have greater control over a URL... That said, it's never our intention to miss a use case!

Feel free to submit a PR if you've got the time? We're happy to take a look.

oscarleonnogales commented 7 months ago

Hi @rafaelguedes, thank you for reaching out over this particular case. After reviewing this, the example you provided represents a unique case where a benign string is concatenated with a potentially dangerous protocol. Our current sanitization approach tries to balances security against malicious inputs with the need to preserve functionality for legitimate URLs.

We check where these URL protocols appear. If they are part of the query parameters, fragments, or within a larger string that does not represent a URL scheme, they are considered safe. Here's the test that ensures this particular case makes it through.

We appreciate your input and encourage you to share any further concerns or specific patterns you believe we should address. Thank you again for your contribution, as this helps us maintain the quality and security of our library.