1357369 - docshell should strip wyciwyg: before calling ContentPolicies

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P3)

Description

[anandtapangupta68]

Attachment: index.html

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170323105023

Steps to reproduce:

1. Serve the attached file index.html from a server
2. Open it in the browser
2. Press the start test button.


Actual results:

1. The test creates an iframe and injects content into it using document.write
2. The content tries to display an image which is not allowed in parent's CSP.
3. After setting its own sandbox attribute to "allow-scripts", the iframe can bypass the CSP that is applied by the parent. 


Expected results:

The chil iframe should be subject to the CSP of the parent if it created using document.write.

Comment 1

[Alice0775 White]

@reporter,
Could you test this with [1]beta53, Developer Edition54 and nightly55?

[1]
https://www.mozilla.org/en-US/firefox/channel/desktop/

Comment 2

[Loic]

In FF53+, the image in the child is blocked (but the word "child" still appears).

Comment 3

[Alice0775 White]

So, it seems to be fixed in FF53+.

Comment 4

[Alice0775 White]

seems fixed by Bug 1329288

Comment 5

[anandtapangupta68]

Tested on Firefox Nightly 55.0a1 (2017-04-07) (64-bit)
The image is not loaded, but I see a weird looking CSP violation message:

Content Security Policy: The page’s settings blocked the loading of a resource at wyciwyg://4/http://localhost:8000/Tests/cspBypasses/docWriteWithSandbox/ (“frame-src http://localhost:8000”)

Is Firefox giving some artificial origin to document.write iframes?

Comment 6

[Alice0775 White]

(In reply to anandtapangupta68 from comment #5)
> Tested on Firefox Nightly 55.0a1 (2017-04-07) (64-bit)
> The image is not loaded, but I see a weird looking CSP violation message:
> 
> Content Security Policy: The page’s settings blocked the loading of a
> resource at
> wyciwyg://4/http://localhost:8000/Tests/cspBypasses/docWriteWithSandbox/
> (“frame-src http://localhost:8000”)
> 
> Is Firefox giving some artificial origin to document.write iframes?


@:ckerschb,
What do you think about that?
It seems a bug.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

Let my try to summarize everything that maybe has already been said and what not:

First, the problem reported in comment 0 is not a problem anymore from FF53 onwards. I am not sure if Bug 1329288 (as reported in comment 4) has fixed the issue - I would rather guess that Bug 1182569 is the bug that actually stopped this problem from occurring.

The reload() in the testcase actually causes us to use the wyciwyg (-> what you cache is what you get) channel, which is correct from a platform point of view, but we shouldn't display it in the webconsole, or use that URI when calling ContentPolicies. Within the ContentSecurityManager we already doing the right thing and stripping the wyciwyg channel [1]. Please note that the wyciwg applies only to docshell loads (toplevel, and [i]frame loads). Within [2] we are actually going to remove the contentPolicy check from docshell, which should then also display the right information in the webconsole, because the second contentPolicy check is performed here [1].
In case someone wants to fix that, should use a patch similar to what I will post inline undeneath (haven't compiled that version though, so might need some tweeking).

[1] https://dxr.mozilla.org/mozilla-central/source/dom/security/nsContentSecurityManager.cpp#186
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1331740#c1

diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -9945,16 +9945,25 @@ nsDocShell::InternalLoad(nsIURI* aURI,
     // argument to pass a specific identifier.
     nsCOMPtr<nsISupportsString> extraStr =
       do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
     NS_NAMED_LITERAL_STRING(msg, "conPolCheckFromDocShell");
     rv = extraStr->SetData(msg);
     NS_ENSURE_SUCCESS(rv, rv);
 
+    nsCOMPtr<nsIURIFixup> urifixup(do_GetService(NS_URIFIXUP_CONTRACTID, &rv));
+    if (NS_SUCCEEDED(rv) && urifixup) {
+      nsCOMPtr<nsIURI> fixedURI;
+      rv = urifixup->CreateExposableURI(uri, getter_AddRefs(fixedURI));
+      if (NS_SUCCEEDED(rv)) {
+        aURI = fixedURI;
+      }
+    }
+
     int16_t shouldLoad = nsIContentPolicy::ACCEPT;
     rv = NS_CheckContentLoadPolicy(contentType,
                                    aURI,
                                    aTriggeringPrincipal,
                                    requestingContext,
                                    EmptyCString(),  // mime guess
                                    extraStr,  // extra
                                    &shouldLoad);

Comment 8

[anandtapangupta68]

One thing that concerns me most is the error shows frame-src in the violation message, while I would expect the image being blocked due the default-src directive (since img-src is not present). Can somebody clarify what is exactly going on?

Comment 9

[Christoph Kerschbaumer [:ckerschb]]

(In reply to anandtapangupta68 from comment #8)
> One thing that concerns me most is the error shows frame-src in the
> violation message, while I would expect the image being blocked due the
> default-src directive (since img-src is not present). Can somebody clarify
> what is exactly going on?

Sorry, for not being precise enough. When running the test using the latest code on mozilla-central, I get three messages on the browser console:

> Content Security Policy: Directive ‘frame-src’ has been deprecated. Please use directive ‘child-src’ instead. (unknown)
> Content Security Policy: The page’s settings blocked the loading of a resource at https://www.w3schools.com/html/html5.gif (“default-src 'none'”).  (unknown)
> Content Security Policy: The page’s settings blocked the loading of a resource at wyciwyg://0/https://bug1357369.bmoattachments.org/attachment.cgi?id=8859127 (“frame-src https://bug1357369.bmoattachments.org”).  (unknown)

The first indicates that frame-src is deprecated, the second refers to the blocked image and the third would disappear if we would land the code snippet in comment 7.

Comment 10

[anandtapangupta68]

Okay, so in that case only the third message is the problem

Comment 11

[Christoph Kerschbaumer [:ckerschb]]

(In reply to anandtapangupta68 from comment #10)
> Okay, so in that case only the third message is the problem

correct.

Comment 12

[Andrew McCreight [:mccr8]]

Bug 1489308 will get rid of wyciwyg URLs, which I think will remove the need for the code in comment 7.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Yep. Fixed by bug 1489308.