Add option to allow empty URL schemes

[lukebarnard1]

Set to true by default, this can be set to false to avoid //evil.com/nasty or evil.com/nasty being allowed as hrefs.

[boutell]

For protocol-relative URLs, I think it would make more sense to have an option to specify what the default scheme is (what a protocol-relative URL is effectively using), and allow or disallow it as any other URL of that scheme would be. I don't think your second case ( "some.domain.co/nasty" ) is a real thing — that would prevent the use of a subdirectory by that name, which would be a very strange thing for a URL parser to do. But you're right that we have to do something intelligent with protocol-relative URLs (starting with // ).

…

On Wed, Jan 11, 2017 at 12:46 PM, Luke Barnard ***@***.***> wrote: Set to true by default, this can be set to false to avoid //evil.com/nasty or evil.com/nasty being allowed as hrefs. ------------------------------ You can view, comment on, or merge this pull request online at: #137 Commit Summary - Add option to allow empty URL schemes File Changes - *M* index.js <https://github.com/punkave/sanitize-html/pull/137/files#diff-0> (7) Patch Links: - https://github.com/punkave/sanitize-html/pull/137.patch - https://github.com/punkave/sanitize-html/pull/137.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#137>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB9fYa2HxgBZHWbUC3q9-6moINJohAOks5rRRV6gaJpZM4Lg3jr> .

-- *THOMAS BOUTELL, *SUPPORT LEAD P'UNK AVENUE | (215) 755-1330 | punkave.com

[lukebarnard1]

For protocol-relative URLs, I think it would make more sense to have an option to specify what the default scheme is

Sounds like a good idea. When the default scheme has not been provided, should protocol-relative URLs be rejected as nasty? i.e.:

var matches = href.match(/^([a-zA-Z]+)\:/);
if (!matches) {
  // No scheme, "//nasty.domain.co" or "nasty.domain.co"
  if (!options.defaultScheme || typeof options.defaultScheme !== 'string') {
    return false;
  }
  // Match against default scheme
  return href.startsWith(options.defaultScheme);
}

I don't think your second case ( "some.domain.co/nasty" ) is a real thing

Sorry, I should've made it obvious that it's the some.domain.co bit which is the nasty bit.

[boutell]

On reflection, I think I'm wrong. You don't always know what the scheme will be when you're sanitizing.

Your original patch is the right idea, however you need to actually verify that the URL starts with // before treating it as protocol-relative. It looks like your code would assume all relative URLs are protocol-relative URLs.

[lukebarnard1]

Would it not be best to also have an option to [dis]allow the protocol-less some.evil.com? Most browsers would allow no "//" to be treated as an implicit "//".

[boutell]

No, they don't (proof welcome though).

…

On Fri, Jan 13, 2017 at 12:12 PM, Luke Barnard ***@***.***> wrote: Would it not be best to also have an option to [dis]allow the protocol-less some.evil.com? Most browsers would allow no "//" to be treated as an implicit "//". — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#137 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB9ffU0HO5EnoxjXp0biV3eMkW80tPfks5rR7BogaJpZM4Lg3jr> .

-- *THOMAS BOUTELL, *SUPPORT LEAD P'UNK AVENUE | (215) 755-1330 | punkave.com

[lukebarnard1]

Ah. My mistake. I shall update the comment!

I guess it should also return false if there is no scheme?

[boutell]

This has been published to npm after reversing that if statement (naughtyHref returns true for bad things, not happy things). Thanks!