401343 - restrict web-based protocol handlers to http/https

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[Jeff Walden [:Waldo]]

Attachment: Proof of concept, works with a little setup

I shouldn't be able to register a URI as a content handler to which I couldn't create a link.

URL encoding makes this a bit tricky to actually make work, but if you can find a file whose name contains a colon, you can load it by using the newly-registered protocol.  Stuff like "file:///bin/rm -rf %s" falls afoul of the encoding restriction.  The example given here works for me if I create a file named "test:bar" in the right location.

I tend to think we should go beyond "create a link" to "is a URL from a specific blacklist of protocols", because this works partly because file: URL objects have a non-throwing .host property, and I wouldn't trust that to always hold for anything we care about (also extensions as well).

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

At the security review for the web-based protocol handling, we tentatively decided to go one better and restrict it to http: or https: URIs until such time as someone comes up with a compelling use case for opening it up more.  We might want to suggest this to the WhatWG as well.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Looks like the registerContentHandler used by Firefox already has this restriction; we only really need to fix registerProtocolHandler.  But see also bug 401535 about how this logic really shouldn't need to be implemented once per frontend.

Comment 3

[Florian Quèze [:florian]]

Attachment: patch v1

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 287748 [details] [diff] [review]
patch v1

Native callers can optimize by using schemeIs rather than getting scheme and doing a compare, but since this is going through xpconnect either way it probably doesn't matter much.

Comment 5

[Reed Loden [:reed]]

Checking in browser/components/feeds/src/WebContentConverter.js;
/cvsroot/mozilla/browser/components/feeds/src/WebContentConverter.js,v  <--  WebContentConverter.js
new revision: 1.24; previous revision: 1.23
done