braintree / sanitize-url

MIT License
312 stars 35 forks source link

Protocol-less network paths return 'about:blank' #18

Closed chawes13 closed 4 years ago

chawes13 commented 4 years ago

When sanitizing a user's input for their website, I was expecting a protocol-less url without XSS vectors to return the user's input.

Expected:

import { sanitizeUrl } from '@braintree/sanitize-url'

sanitizeUrl('www.google.com') // www.google.com
sanitizeUrl('google.com') // google.com

Actual:

import { sanitizeUrl } from '@braintree/sanitize-url'

sanitizeUrl('www.google.com') // about:blank
sanitizeUrl('google.com') // about:blank

The documentation via the tests seems to suggest that a protocol-less url should be acceptable. However, the isUrlWithoutProtocol helper requires a relative path (i.e., '.' or '/').

crookedneighbor commented 4 years ago

@chawes13 This module is used primarily with the Braintree SDKs, so it was built to accommodate the kinds of urls that pass through them (web urls with protocols, as well as deep link mobile urls).

If you'd like to open a PR to account for the behavior you pointed out, we're happy to review it, but it meets the needs of the Braintree SDKs as is, so I don't see us changing it ourselves in the near future. (too many other more important tasks to handle right now)

chawes13 commented 4 years ago

That's fair! Appreciate the quick response. I'll likely just fork for our immediate needs and then submit a PR in a few weeks if I can settle on an approach.

crookedneighbor commented 4 years ago

:+1:

I'm going to close this issue for now, since there's nothing actionable for the Braintree team, but definitely open a PR. We're happy to look at it.