Bug 59850

Summary: CSP script-src should block eval
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dglazkov, eric, ggaren, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 53572    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Adam Barth 2011-04-29 17:37:10 PDT 
CSP script-src should block eval
Comment 1 Adam Barth 2011-04-29 17:41:18 PDT 
Created attachment 91773 [details]
Patch
Comment 2 Adam Barth 2011-04-29 17:43:00 PDT 
Here's the related V8 bug:
http://code.google.com/p/v8/issues/detail?id=1258
Comment 3 Eric Seidel (no email) 2011-04-29 17:44:13 PDT 
Comment on attachment 91773 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=91773&action=review

> LayoutTests/http/tests/security/contentSecurityPolicy/eval-blocked.html:11
> +This test passes if it doesn't alert fail.

This is confusing.

> Source/JavaScriptCore/runtime/Executable.cpp:106
> +        return throwError(exec, createEvalError(exec, "Eval is disabled"));

Is this the right text?

> Source/JavaScriptCore/runtime/JSGlobalObject.h:115
> +        bool m_isEvalEnabled : 1;

Do we worry about the size of this object?
Comment 4 WebKit Review Bot 2011-04-29 17:45:31 PDT 
Attachment 91773 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8517974
Comment 5 Adam Barth 2011-04-29 17:45:47 PDT 
> > LayoutTests/http/tests/security/contentSecurityPolicy/eval-blocked.html:11
> > +This test passes if it doesn't alert fail.
> 
> This is confusing.

Maybe:

This test passes if it doesn't alert "fail."

?

> > Source/JavaScriptCore/runtime/Executable.cpp:106
> > +        return throwError(exec, createEvalError(exec, "Eval is disabled"));
> 
> Is this the right text?

There's no spec for JavaScript errors.

> > Source/JavaScriptCore/runtime/JSGlobalObject.h:115
> > +        bool m_isEvalEnabled : 1;
> 
> Do we worry about the size of this object?

Dunno.  The object is very large.  I could remove the ": 1".
Comment 6 Adam Barth 2011-04-29 17:48:26 PDT 
Created attachment 91775 [details]
Patch
Comment 7 Adam Barth 2011-04-29 18:13:56 PDT 
Created attachment 91783 [details]
Patch
Comment 8 WebKit Commit Bot 2011-04-29 21:16:11 PDT 
The commit-queue encountered the following flaky tests while processing attachment 91783 [details]:

http/tests/xmlhttprequest/failed-auth.html bug 51835 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.
Comment 9 WebKit Commit Bot 2011-04-29 21:30:16 PDT 
Comment on attachment 91783 [details]
Patch

Clearing flags on attachment: 91783

Committed r85388: <http://trac.webkit.org/changeset/85388>
Comment 10 WebKit Commit Bot 2011-04-29 21:30:22 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 WebKit Commit Bot 2011-04-29 23:15:53 PDT 
The commit-queue encountered the following flaky tests while processing attachment 91783 [details]:

http/tests/xmlhttprequest/cross-origin-authorization.html bug 52398 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.