548984 - CSP javascript: URL restrictions need to be unified under "options inline-script"

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect)

Description

[Brandon Sterne (:bsterne)]

We have code in nsJSProtocolHandler to block javascript: URLs when a link is clicked.

We have a nsIContentPolicy implementation to blocks javascript: URIs for content loads such as:
<iframe src="javascript:alert(1)"></iframe>

But if a site says:
X-Content-Security-Policy: allow 'self' javascript:

and tries to include:
<iframe src="javascript:alert(1)"></iframe>

the load will be blocked.

JS URL content loads should only be restricted by the nsIContentPolicy and options inline-script should only apply to JS link clicks.

Comment 1

[Daniel Veditz [:dveditz]]

I think the opposite: it would be better (more consistent, less confusing) to drop "javascript:" support in content load directives and simply control all use of javascript: from the allow-inline option. Implementation-wise the content-policy would special-case javascript--which is definitely a special case--and check the allow-inline option when it sees one.

What is the benefit of splitting these two?

Comment 2

[Brandon Sterne (:bsterne)]

I don't think there's a security benefit to splitting them from a policy syntax perspective.  A user can get just as pwned by clicking a malicious javascript: link as they can loading a javascript: URI.

The reason I filed the bug, though, is that we have to block them separately already so I thought maybe we could expose that functionality via policy to sites that wanted to differentiate the two cases.  Upon reconsideration, I agree this is too confusing and we should stick to a single policy mechanism to globally allow or disallow javascript: URIs.

The bug, then, is that
X-CSP: allow 'self'; options inline-script

prevents the <iframe src="javascript:alert(1)"></iframe> case from working.

We need to make the parser notify our nsIContentPolicy that javascript: URIs are permitted when options inline-script is present.

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

Should be a pretty simple patch... just have to special case for "javascript:" URIs that come into the content policy.  Here's pseudocode:

 if (aURI.schemeIs("javascript")) {
   return cspObj->allowsInline ? 
          nsIContentPolicy.ALLOW : 
          nsIContentPolicy.REJECT_SERVER;
 }
 // existing content policy stuff

We may also want to pop a warning into the JS console when parsing a policy that contains "javascript:" as a source in any directive -- this would let developers know that the "javascript:" part of their policy will be ignored.

We should also make it clear in the spec that "javascript:" is ignored as a source in the various content directives.

Comment 4

[Sid Stamm [:geekboy or :sstamm]]

Seems to me that if we consider javascript: URIs to be inline scripts, we should filter/block them at compile time, not at request time.  This means the nsIContentPolicy should allow them and the nsJSProtocolHandler should take care of the filtering based on the script's inherited principal.

Comment 5

[mhfrltza]

This bug also appears to prevent iframes from the same origin and with the same CSP behaving as would be expected. 

For example, the page behaves differently when within an iframe compared to when that same frame is opened in a new window. In the test script I posted on bug 608131 it is the same php script inside the frame, but javascript: links do not work in the frame but do work in the parent.

Comment 6

[Sid Stamm [:geekboy or :sstamm]]

(In reply to mhfrltza from comment #5)
> This bug also appears to prevent iframes from the same origin and with the
> same CSP behaving as would be expected. 

Bug 608131 was filed for that type of issue, and is blocked by this one.

Comment 7

[Tanvi Vyas[:tanvi]]

Steps for this bug:

1) We need to allow javascript: in nsIContentPolicy (the way chrome: is allowed):
 if (aURI.schemeIs("javascript")){
    nsIContentPolicy.ALLOW
 }

2) Then we need to check if options inline-script is set in nsJSProtocolHandler, and allow javascript: if inline-script is allowed, and reject it otherwise.

3) We may also want to pop a warning into the JS console when parsing a policy that contains "javascript:" as a source in any directive -- this would let developers know that the "javascript:" part of their policy will be ignored.  It is covered by in-line scripts.

4) We should also make it clear in the spec that "javascript:" is ignored as a source in the various content directives.

Comment 8

[Tanvi Vyas[:tanvi]]

> 1) We need to allow javascript: in nsIContentPolicy (the way chrome: is
> allowed):
>  if (aURI.schemeIs("javascript")){
>     nsIContentPolicy.ALLOW
>  }

We can add this code to nsContentUtils.cpp in the URIIsChromeOrInPref(nsIURI *aURI, const char *aPref) function.

This function is called two times.  Once in IsWhiteListed() in nsScreen.cpp and once in BrowserFrameSecurityCheck() in nsGenericHTMLFrameElement.cpp.  Allowing javascript in addition to chrome seems to make sense here.  However, perhaps we should change the function name, since javascript is neither Chrome or in the Pref.

In order to make progress on this bug, I'm not going to change the function name for now. I'll wait until I have a patch and see what the feedback providers/reviewers say.

Comment 9

[Boris Zbarsky [:bzbarsky]]

I'd really appreciate people not hardcoding "javascript".  You want either URI_OPENING_EXECUTES_SCRIPT or URI_INHERITS_SECURITY_CONTEXT, I suspect, depending on what you really think is special about "javascript".

Comment 10

[Tanvi Vyas[:tanvi]]

Thanks Boris for the note!  I'll take a look at URI_OPENING_EXECUTES_SCRIPT and URI_INHERITS_SECURITY_CONTEXT and use one of them instead of "javascript".

Comment 11

[Tanvi Vyas[:tanvi]]

Looking at both URI_OPENING_EXECUTES_SCRIPT and URI_INHERITS_SECURITY_CONTEXT and their usage in the code, it seems that the URI_OPENING_EXECUTES_SCRIPT is what we are looking for here.


/* URIs for this protocol execute script when they are opened. */
const unsigned long URI_OPENING_EXECUTES_SCRIPT = 8192


/*  The URIs for this protocol have no inherent security context, so
 documents loaded via this protocol should inherit the security context
 from the document that loads them. */
const unsigned long URI_INHERITS_SECURITY_CONTEXT = 16

Comment 12

[Ian Melven :imelven]

Just a note that this bug may get addressed for 1.0 compliant CSP policies at least via the work in bug 746978, which takes the approach Sid suggested in comment 4 of this bug.

Comment 13

[Ian Melven :imelven]

This will be fixed when bug 746978 lands, see comment 61 in that bug.

Comment 14

[Ian Melven :imelven]

This probably is fixed by the fix to bug 820196, see https://hg.mozilla.org/mozilla-central/diff/30de61bc8321/content/base/src/nsCSPService.cpp

I'll verify if this is the case.

Comment 15

[Sid Stamm [:geekboy or :sstamm]]

Attachment: example for manual test

Here's a file to manually test whether a build allows inline scripts or not.  With the CSP including "options inline-script", javascript URIs should work.  Without that option, javascript URIs should be blocked (even if javascript: is included in script-src).

Comment 16

[Sid Stamm [:geekboy or :sstamm]]

Using the attached test file, I verified this is fixed in Firefox 19.0.2.

I'm not automating the test since the parser used for testing this will be replaced by our new parser soon (see bug csp-w3c-1.0) and this bug was fixed as part of implementing that new parser (bug 746978).

Also, closing this bug since it's incidentally fixed by another patch.