1056882 - HTML import polyfill bypasses natural CSP restrictions

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Components, defect)

Description

[Frederik Braun [:freddy]]

As specified in the imports spec, HTML imports are subject to script-src CSP restrictions. For gaia apps, this means disallowing data: URIs for example.

The html polyfill does *not* check and allows everything that it can get via XHR (i.e., same-origin or CORS enabled - both fine).

But the HTML import polyfill can *not* bypass the "inline script" restriction which is given through the CSP that is in place AFAIU.

Comment 1

[Frederik Braun [:freddy]]

Kevin are you a good owner for this (I guessed with git blame)?
I wonder if there's a good way to check for bad origins using window.URL.

Comment 2

[Frederik Braun [:freddy]]

P.S: This means that once we move to the platform version, things *might* break. I think it's rather unlikely though.

CCing Gabor, who works on the imports implementation on the platform side. He might be interested in this.

Comment 3

[Kevin Grandon :kgrandon]

I'm not too familiar with the inner workings of CSP. Does it not work when we set innerHTML?

Comment 4

[Frederik Braun [:freddy]]

When the polyfill sets innerHTML the scripts that may come with it are subject to CSP and that's fine.

The problem is a bit more theoretical: imports (as specified) should look at the script-src directive and disallow importing documents that are not whitelisted as "script-src" (Gaia allows only 'self', i.e. the current origin).

The polyfill however does not look at the policy. Instead it imports from everywhere it can get because it uses XMLHttpRequest (XHR). This would allow importing dynamic content via data URIs.


Thinking about this further, it's probably only sec-low ;)

Comment 5

[Frederik Braun [:freddy]]

So what we can not fix is that the polyfill can not look at the CSP. If the CSP is very specific about script-src, it would provide a CSP bypass.

Fortunately the policy is the same for all privileged and certified apps (which means we disallow reading scripts from the web).

What I suggest is a small patch for html_imports.js to disallow linking to other schemes.

Comment 6

[Frederik Braun [:freddy]]

Attachment: only same-origin imports

Can you review this patch or find someone else to look at it, Kevin?

Comment 7

[Frederik Braun [:freddy]]

Note to self: This won't need sec-approval because it's sec-low. Putting it on Github.

Comment 8

[Frederik Braun [:freddy]]

Attachment: only same-origin imports, as github PR

The same patch, as a github pull request.

Comment 9

[Frederik Braun [:freddy]]

If this is generally approved, I'll spin up a next iteration: "document.baseURI" could be DOM-clobbered[1]. It's better to use location.href :)

[1] http://www.thespanner.co.uk/2013/05/16/dom-clobbering/

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8478898 [details] [review]
only same-origin imports, as github PR

Seems fine to me, might be worth adding a comment above the code though. Thanks!

Comment 11

[Frederik Braun [:freddy]]

Updated the patch with a comment "bail out if the imported resource isn't in the same origin".
Now using location.href instead of document.baseURI.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/503d3c4e4c779f31ccb768a8ccc2b505aff72e83

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to revert this for linter test failures: https://github.com/mozilla-b2g/gaia/commit/6e804a42ab90f4251c7fe8c68731dc1c6abd8006

https://tbpl.mozilla.org/php/getParsedLog.php?id=46799653&tree=B2g-Inbound

Comment 14

[Frederik Braun [:freddy]]

Attachment: only same-origin imports, as github PR (addressing linter errors)

linter was green on a local "make lint".

Comment 15

[Gabor Krizsanits (INACTIVE)]

What about CORS?

Comment 16

[Frederik Braun [:freddy]]

Good catch, but we strictly don't allow CORS in FxOS privileged and certified apps: The Content Security Policy disallows script loads from anything but "self".

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/d281d90c3679d513627e17c0f4dcf9e895c9b47d