Bug 111867

Summary: CSP: 'eval()' is blocked in report-only mode.
Product: WebKit Reporter: Mike West <mkwst>
Component: WebCore Misc.Assignee: Mike West <mkwst>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, mkwst+watchlist, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=138492
Bug Depends on:    
Bug Blocks: 111869    
Attachments:
Description Flags
Patch none

Mike West
Reported 2013-03-08 10:03:48 PST
In report-only mode, CSP should allow 'eval()' to execute; it should not be blocked. That's not what we're currently doing. See http://stackoverflow.com/questions/15273841/eval-isnt-executed-in-chrome-when-using-content-security-policy-report-only/15300012#15300012.
Attachments
Patch (5.28 KB, patch)
2013-03-08 10:15 PST, Mike West 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mike West
Comment 1 2013-03-08 10:15:17 PST
Created attachment 192248 [details] Patch
Adam Barth
Comment 2 2013-03-08 11:52:05 PST
Comment on attachment 192248 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192248&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:-1460 > - if (!allowEval(0, SuppressReport)) > - m_scriptExecutionContext->disableEval(evalDisabledErrorMessage()); Why did you move this into the loop? It seems like we can just add a check for |type| to this code here.
Mike West
Comment 3 2013-03-08 11:57:46 PST
(In reply to comment #2) > (From update of attachment 192248 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192248&action=review > > > Source/WebCore/page/ContentSecurityPolicy.cpp:-1460 > > - if (!allowEval(0, SuppressReport)) > > - m_scriptExecutionContext->disableEval(evalDisabledErrorMessage()); > > Why did you move this into the loop? It seems like we can just add a check for |type| to this code here. I moved it into the loop because it was the simplest mechanism for checking whether a specific policy was report-only, as opposed to checking if the combined set of policies allowed eval or not. We don't currently have a mechanism of distinguishing between these two cases: * Content-Security-Policy: script-src 'self'; and * Content-Security-Policy-Report-Only: script-src 'self'; * Content-Security-Policy: img-src 'none'; For most directives, this doesn't matter, as we do the report-only check inline with the request. For eval, on the other hand, we have to tell V8/JSC whether it should allow or deny 'eval()' a priori. See also bug 111869 for another consequence of this. I think the behavior of ContentSecurityPolicy::allowEval should probably change, as you suggest, but I'm not sure how that needs to look
Adam Barth
Comment 4 2013-03-08 12:16:19 PST
Doesn't the HeaderType argument tell you?
Adam Barth
Comment 5 2013-03-08 12:18:15 PST
Comment on attachment 192248 [details] Patch I understand now. After the loop we're querying all previous headers too.
WebKit Review Bot
Comment 6 2013-03-08 14:21:58 PST
Comment on attachment 192248 [details] Patch Clearing flags on attachment: 192248 Committed r145268: <http://trac.webkit.org/changeset/145268>
WebKit Review Bot
Comment 7 2013-03-08 14:22:02 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.