Closed Bug 882060 Opened 11 years ago Closed 11 years ago

A policy of like script-src 'self' 'unsafe-inline'; allows eval but should not

Categories

(Core :: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: freddy, Assigned: geekboy)

References

(Blocks 1 open bug, )

Details

(Keywords: sec-moderate)

I ran the W3C test suite (see URL) and noticed that we execute JavaScript from strings when script-src is set to 'self 'unsafe-inline', although unsafe-eval is left out.
Thanks for running the test suite, Freddy ! 

We have some tests around setInterval/setTimeout, see https://mxr.mozilla.org/mozilla-central/source/content/base/test/file_CSP_evalscript_main.js

so we need to dig in a bit to understand what's going on here and what's different (or possibly what's wrong in our tests for this). 

Marking sec-moderate, as always, that's open to discussion.

I don't have time to look into this right now, so if anyone else can dive in that would be much appreciated. This test passes in current Chrome fwiw.
Keywords: sec-moderate
We execute all types of JS from strings or just one certain vector?  Is it inline script tags or event handler attributes or...?
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #2)
> We execute all types of JS from strings or just one certain vector?  Is it
> inline script tags or event handler attributes or...?

Fail	setInterval, setTimeout with non-callable args should not execute with policy "script-src 'self' 'unsafe-inline'". 1	assert_false: Unsafe eval ran in setTimeout(). expected false got true
Fail	setInterval, setTimeout with non-callable args should not execute with policy "script-src 'self' 'unsafe-inline'". 2	assert_false: Unsafe eval ran in setInterval(). expected false got true

the script is :

<script>
		window.setTimeout('test(function() {assert_false(true, "Unsafe eval ran in setTimeout().")})', 0);
	</script>
	<script>
		window.setInterval('test(function() {assert_false(true, "Unsafe eval ran in setInterval().")})', 0);
	</script>


note that the CSP for the document specifies unsafe-inline, just not unsafe-eval
Thanks, Ian.  Potentially (remotely) related is bug 805929.
Looks like we don't have a setInterval test :(

Wondering if this has something to do with the W3C test using window.setTimeout, our test just calls setTimeout. Very likely not, but something I noticed.
"window." shouldn't make a difference.  I just tried it out and doesn't seem to affect the outcome of the mochitests.
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #6)
> "window." shouldn't make a difference.  I just tried it out and doesn't seem
> to affect the outcome of the mochitests.

Didn't think so, thanks for checking. Seems like the test case needs some debugging, if we can find a volunteer..
Assignee: nobody → sstamm
From my quick tests, the issue appears only when "Content-Security-Policy" is used, rather than "X-Content-Security-Policy". I'm using FF24 nightly.
These tests which were failing previously :

Fail	setInterval, setTimeout with non-callable args should not execute with policy "script-src 'self' 'unsafe-inline'". 1	assert_false: Unsafe eval ran in setTimeout(). expected false got true
Fail	setInterval, setTimeout with non-callable args should not execute with policy "script-src 'self' 'unsafe-inline'". 2	assert_false: Unsafe eval ran in setInterval(). expected false got true

are now passing in Nightly Fx25 from 7/1. I suspect this is due to Garrett's fix for bug 887974 having landed.

There is still a report sending failure but that might be bug 843311.
I just ran the entire test suite on nightly (http://webappsec-test.info/web-platform-tests/csp/) and got 100%.  Some of the php files don't seem to have any tests, but when I load them individually they block some stuff.

Freddyb: is this bug is fixed on nightly?
Flags: needinfo?(fbraun)
This eval/inline failure seems to be fixed and I do see a success rate of 100%. Thanks for working on this Garett!

But there seem to be some outstanding bugs anyhow: Firefox nightly sees a 100% pass with nine of nine tests, where chromium sees and passes 16 tests. This should be investigated in bug 937025.
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(fbraun)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.