Closed
Bug 887974
Opened 11 years ago
Closed 11 years ago
CSP: when script-src has both 'unsafe-inline' and 'unsafe-eval' directives present, eval() is still not allowed
Categories
(Core :: General, defect)
Tracking
()
VERIFIED
FIXED
mozilla25
People
(Reporter: bcopeland, Assigned: grobinson)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
geekboy
:
review+
bajaj
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20130627 Firefox/25.0 (Nightly/Aurora)
Build ID: 20130627031027
Steps to reproduce:
I'm trying to deploy CSP-1.0 compliant headers where we have used X-CSP in the past. On Aurora as of today, 6/27, I cannot get unsafe-eval to work. (Yes, don't do that, but still...)
Simple test case - php script:
-----------------------------------------------------------------
<?php
header("Content-Security-Policy: default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'");
?>
<script>
// this works:
//alert("hello world!");
// this fails (works in chrome, unless unsafe-eval is removed)
eval('alert("hello world!")');
</script>
-----------------------------------------------
wget -S shows header being set as:
Content-Security-Policy: default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'
Actual results:
[15:52:03.217] Error: call to eval() blocked by CSP @ https://[...]:5
[15:52:03.246] Content Security Policy: Directive eval script base restriction violated @ https://[...]:5
Expected results:
Alert worked.
Updated•11 years ago
|
Flags: needinfo?(sstamm)
Comment 1•11 years ago
|
||
Garrett has a fix for this, we just spotted the bug together while working on bug 885433 and debugging the test he wrote for that.
Assignee: nobody → grobinson
Flags: needinfo?(sstamm)
Comment 2•11 years ago
|
||
Thank you for reporting this, Bob !
Updated•11 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 3•11 years ago
|
||
Can *you* spot the bug? Props to imelven. This also fixes the weird behavior I was seeing in my Mochitest 885433.
Attachment #768672 -
Flags: review?(sstamm)
Comment 4•11 years ago
|
||
This bug only manifests when a policy contains BOTH 'unsafe-inline' *and* 'unsafe-eval' btw.
Comment 5•11 years ago
|
||
Comment on attachment 768672 [details] [diff] [review]
Patch 1
Review of attachment 768672 [details] [diff] [review]:
-----------------------------------------------------------------
Nit: you didn't take long enough to fix this bug. I tested the fix locally with the PHP test case provided, r=me.
Attachment #768672 -
Flags: review?(sstamm) → review+
Comment 6•11 years ago
|
||
The test Garrett has written for bug 885433 specifies both 'unsafe-inline' and 'unsafe-eval' which is how we found the issue. That should land shortly (we want to get bug 886132 fixed) and then we will have a test in the suite to cover this case going forward.
Updated•11 years ago
|
Keywords: checkin-needed
Comment 7•11 years ago
|
||
We will almost definitely want to nominate this for uplift to Fx24 (Aurora) and Fx23 (Beta).
status-firefox23:
--- → affected
status-firefox24:
--- → affected
tracking-firefox23:
--- → ?
tracking-firefox24:
--- → ?
Comment 8•11 years ago
|
||
Updated•11 years ago
|
Keywords: checkin-needed
Updated•11 years ago
|
Summary: CSP 'unsafe-eval' directive ignored → CSP: when script-src has both 'unsafe-inline' and 'unsafe-eval' directives present, eval() is still not allowed
Updated•11 years ago
|
Blocks: csp-w3c-1.0
Comment 9•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment 10•11 years ago
|
||
Comment on attachment 768672 [details] [diff] [review]
Patch 1
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 746978
User impact if declined: CSP policies with script-src: 'unsafe-inline' 'unsafe-eval' will not work correctly (eval will not be allowed when it should be)
Testing completed (on m-c, etc.): This just landed but it's a small change - there's a lot of CSP tests to minimize regression worry. Comment 6 explains that grobinson is going to add a test that will cover the case of a policy with script-src: 'unsafe-inline' 'unsafe-eval' very shortly
Risk to taking this patch (and alternatives if risky): should be low, see above re testing
String or IDL/UUID changes made by this patch: nope
We will probably want to uplift this to beta too.
Attachment #768672 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
status-firefox22:
--- → unaffected
status-firefox25:
--- → fixed
Updated•11 years ago
|
Updated•11 years ago
|
Attachment #768672 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 11•11 years ago
|
||
Thanks, Bhavana !
https://hg.mozilla.org/releases/mozilla-aurora/rev/3f9d6dbaf669
How long should we wait before nominating for beta uplift ? This is a pretty bad bug with our CSP 1.0 support and it would be good to get it fixed in Fx23 before that goes to release.
Updated•11 years ago
|
Updated•11 years ago
|
Flags: needinfo?(bbajaj)
Comment 12•11 years ago
|
||
Please nominate for beta uplift so we can get this in next week's builds
Flags: needinfo?(bbajaj)
Updated•11 years ago
|
Flags: needinfo?(grobinson)
Assignee | ||
Comment 13•11 years ago
|
||
Comment on attachment 768672 [details] [diff] [review]
Patch 1
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 887974
User impact if declined: Incorrect, non spec compliant implementation of CSP 1.0
Testing completed (on m-c, etc.): tested with m-c against the CSP test suite
Risk to taking this patch (and alternatives if risky): Minor (only affects CSP)
String or IDL/UUID changes made by this patch: None
Attachment #768672 -
Flags: approval-mozilla-beta?
Updated•11 years ago
|
Attachment #768672 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 14•11 years ago
|
||
Flags: needinfo?(grobinson)
Reporter | ||
Comment 15•11 years ago
|
||
A little late but I can confirm the fix, thanks!
Comment 16•11 years ago
|
||
(In reply to Bob Copeland from comment #15)
> A little late but I can confirm the fix, thanks!
Hi Bob, which versions were you able to test? This should now be fixed in Firefox 23, 24, and 25.
Comment 17•11 years ago
|
||
Bob, can you please let us know which versions you confirm this to be fixed? It should be fixed in Firefox 23, 24, and 25.
Flags: needinfo?(bcopeland)
Reporter | ||
Comment 18•10 years ago
|
||
It was fixed in all versions I could test, and is now deployed on our live site for some time.
Comment 19•10 years ago
|
||
Thanks for getting back to me, Bob. Marking this bug verified fixed based on comment 18.
Status: RESOLVED → VERIFIED
Flags: needinfo?(bcopeland)
You need to log in
before you can comment on or make changes to this bug.
Description
•