Closed Bug 1233875 Opened 9 years ago Closed 8 years ago

Security: CSP is not enforced on programmatically created inline styles and scripts

Categories

(Core :: Security, defect)

43 Branch
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 873302

People

(Reporter: shekyan, Unassigned)

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2593.0 Safari/537.36

Steps to reproduce:

It is possible to have inline styles and javascript event handlers evaluated when CSP is explicitly forbidding. 





Actual results:

Programmatically created style attributes or javascript event handlers are allowed if script that creates them is allowed to be executed.


Expected results:

Should not matter how inline styles or scripts are created for CSP enforcement
Example:

package main

import (
	"fmt"
	"net/http"
)

func main() {
	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Content-Security-Policy", "script-src 'unsafe-inline';style-src")
    fmt.Fprintln(w, `<html>
    <head>
      <title>sss</title>
    </head>
    <body>
    <script>var a = document.createElement('div'); a.style='color:red;'; a.innerHTML = 'should not be red'; document.body.appendChild(a);</script>
    <div style='color:green;'>should not be green</div>
    </body>
  </html>`)
	})
	http.ListenAndServe(":8888", nil)
}
(In reply to shekyan from comment #0)
> Should not matter how inline styles or scripts are created for CSP
> enforcement

That is not the case per spec.

> Example:
> 
> package main
> 
> import (
> 	"fmt"
> 	"net/http"
> )
> 
> func main() {
> 	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
> 		w.Header().Set("Content-Security-Policy", "script-src
> 'unsafe-inline';style-src")

Note that this explicitly allows inline script, and the rest of your example has no event handlers that I can see. So it's unclear what you mean in the title (summary) of this bug report when you say "programmatically created inline...scripts", because this example doesn't have any, and even if it did, you're explicitly allowing them as part of the CSP...

>     fmt.Fprintln(w, `<html>
>     <head>
>       <title>sss</title>
>     </head>
>     <body>
>     <script>var a = document.createElement('div'); a.style='color:red;';
> a.innerHTML = 'should not be red'; document.body.appendChild(a);</script>

Per the spec and numerous bits of documentation ( e.g. https://github.com/blog/1477-content-security-policy#cssom-limitations , http://www.cspplayground.com/compliant_examples#inline-style ) this behaviour working (ie the div's text being red) is correct.

Because this isn't a bug, as best I can tell, opening up and resolving as invalid.
Group: firefox-core-security
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
Component: Untriaged → Security
Product: Firefox → Core
Resolution: --- → INVALID
Uh, 

>That is not the case per spec.

it kind of is. Spec is not really clear on this, as it does not define what 'inline' is.
I put both styles and scripts in the title, but provided the example of the style only.
My example was to demonstrate that a style that I expect to be blocked is not blocked. That particular whitelisted script doesn't matter. The same works with scripts:
package main

import (
	"fmt"
	"net/http"
)

func main() {
	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Content-Security-Policy", "script-src 'nonce-123';")
    fmt.Fprintln(w, `<html>
    <head>
      <title>sss</title>
    </head>
    <body>
      <script nonce='123'>
        var a = document.createElement('div');
        a.innerHTML = 'click me';
        a.onclick = function(){alert(1);};
        document.body.appendChild(a);
      </script>
    </body>
  </html>`)
	})
	http.ListenAndServe(":8888", nil)
}

where while I whitelist the inline script,  I do not whitelist the onclick event handler. And registering the same thing through addEventListener in the same whitelisted script will fail. 


Thanks a lot for pointing to the blog post though. I somehow missed it.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Uh, sorry, addEventListener is also not blocked. But, for example, 

a.innerHTML = '<div onclick=alert(1)>click me</div>';

is blocked,a s expected. 
And in case of style, while 

a.setAttribute('style', 'color:red');

is blocked, 

a.style='color:red;'

is allowed. It is reasonable to expect consistency here.
(In reply to shekyan from comment #3)
> Uh, sorry, addEventListener is also not blocked.

No, otherwise event handling would not be possible on a page where inline script wasn't allowed...

> a.innerHTML = '<div onclick=alert(1)>click me</div>';
> 
> is blocked,a s expected. 

Seems to me that the target of CSP in this case is supposed to be XSS through improper validation of innerHTML and its server-side friends, so this distinction (innerHTML with onclick attribute (new script source!) being blocked, DOM methods to do the same thing from script that's been allowed not being blocked) makes sense to me.

> And in case of style, while 
> 
> a.setAttribute('style', 'color:red');
> 
> is blocked, 
> 
> a.style='color:red;'
> 
> is allowed. It is reasonable to expect consistency here.

Assuming you're still also talking about script: Maybe, but the right place to take that argument is the relevant spec group. It wouldn't make sense for Firefox's behaviour to diverge from other browsers, and would likely break actual web pages.

However... it seems to me that the spec allows:

a.style.color = "red"

but not:

a.style = "color: red"

per point 4 in https://w3c.github.io/webappsec-csp/#directive-style-src , because the former doesn't require parsing CSS declarations (just CSS values) and the latter does. This is what Blink implements, AFAICT (ie the former is allowed, the latter isn't), but Gecko allows both a.style = "color: red" and a.style.color = "red" (again, AFAICT).

CSP 1.0 is pretty clear and just says ( http://www.w3.org/TR/2012/CR-CSP-20121115/#style-src ) "The user agent is also not prevented from applying style from Cascading Style Sheets Object Model (CSSOM)." which matches Gecko's behaviour here. This seems to have changed with CSP 2.0.

And, having just poked about a bit, that issue (not also restricting a.style = "some css text declaration") seems like an exact duplicate of bug 873302.
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.