Bug 93392 – CSP doesn't turn off eval, etc. in Web Workers

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Thomas Sepez]

Originally reported by mazurak [at] cs.wisc.edu at http://code.google.com/p/chromium/issues/detail?id=140544

VULNERABILITY DETAILS
By default, eval and eval-like functionality should be turned off whenever a Content Security Policy is in place, but this isn't happening in workers.  The CSP spec definitely says that the policy on a worker should match the policy on its parent page, and checks on the origins of scripts do appear working correctly.

Comparing SetTimeoutOrInterval in V8WorkerContextCustom.cpp to WindowSetTimeoutImpl in V8DomWindowCustom.cpp, it looks like the necessary checks are just missing on the worker side.

VERSION
Chrome Version: 21.0.1180.57 (stable)
Operating System: Linux (Debian testing) (doesn't appear OS-dependent)

REPRODUCTION CASE
The tests here should all pass for a browser that is implementing CSP and Web Workers correctly (assuming I have not made some silly JavaScript mistakes):

http://pages.cs.wisc.edu/~mazurak/csp_demo/

[Thomas Sepez]

I believe this to be specific to the chromium platform; we'll know once I convert (shortly) the example to layout tests.

[Thomas Sepez]

Created attachment 157478 [details]
Tests, more or less.

[Thomas Sepez]

Created attachment 157570 [details]
Patch, in-progress, fixes for JSC side, missing chromium eval() fix.

[Thomas Sepez]

BTW, not specific to chromium side.

[Eric Seidel (no email)]

This isn't a security bug. :)  It was a conscious FIXME of the time, no need to restrict viewership here.

[WebKit Review Bot]

Comment on attachment 157570 [details]
Patch, in-progress, fixes for JSC side, missing chromium eval() fix.

Attachment 157570 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13492247

New failing tests:
http/tests/security/contentSecurityPolicy/worker-eval-blocked.html
http/tests/security/contentSecurityPolicy/worker-function-function-blocked.html

[WebKit Review Bot]

Created attachment 158110 [details]
Archive of layout-test-results from gce-cr-linux-05

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Thomas Sepez]

Created attachment 158115 [details]
Patch for review.

[WebKit Review Bot]

Comment on attachment 158115 [details]
Patch for review.

Attachment 158115 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13485673

New failing tests:
http/tests/security/contentSecurityPolicy/worker-set-timeout-blocked.html
http/tests/security/contentSecurityPolicy/worker-eval-blocked.html
http/tests/security/contentSecurityPolicy/worker-function-function-blocked.html

[WebKit Review Bot]

Created attachment 158147 [details]
Archive of layout-test-results from gce-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[WebKit Review Bot]

Comment on attachment 158115 [details]
Patch for review.

Attachment 158115 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13484957

New failing tests:
http/tests/security/contentSecurityPolicy/worker-set-timeout-blocked.html
http/tests/security/contentSecurityPolicy/worker-eval-blocked.html
http/tests/security/contentSecurityPolicy/worker-function-function-blocked.html

[WebKit Review Bot]

Created attachment 158156 [details]
Archive of layout-test-results from gce-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Thomas Sepez]

Created attachment 159491 [details]
Patch (including chromium eval() fix).

[Thomas Sepez]

Comment on attachment 159491 [details]
Patch (including chromium eval() fix).

bleh.  Source/WebCore/ChangeLog fell out of the patch :(.

[Thomas Sepez]

Created attachment 159495 [details]
Patch (fix changelog)

[Adam Barth]

Comment on attachment 159495 [details]
Patch (fix changelog)

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

> Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp:249
> +    m_disableEvalPending = !enable;

What if m_context already exists?  Should we call AllowCodeGenerationFromStrings on it?

> Source/WebCore/bindings/v8/WorkerContextExecutionProxy.h:72
> +        void allowEval(bool enable);

setEvalAllowed ?

> Source/WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp:69
> +        if (ContentSecurityPolicy *policy = workerContext->contentSecurityPolicy()) {

ContentSecurityPolicy * -> ContentSecurityPolicy*

[Thomas Sepez]

(In reply to comment #16)
> (From update of attachment 159495 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=159495&action=review
> 
> > Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp:249
> > +    m_disableEvalPending = !enable;
> 
> What if m_context already exists?  Should we call AllowCodeGenerationFromStrings on it?
> 

Nah.  You don't do any work until you call evaluate(), so we just use that one path always.


> > Source/WebCore/bindings/v8/WorkerContextExecutionProxy.h:72
> > +        void allowEval(bool enable);
> 
> setEvalAllowed ?
> 
Will Do.

> > Source/WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp:69
> > +        if (ContentSecurityPolicy *policy = workerContext->contentSecurityPolicy()) {
> 
> ContentSecurityPolicy * -> ContentSecurityPolicy*
Will Do.

[Thomas Sepez]

Created attachment 159506 [details]
Patch (fix nits).

[Adam Barth]

Comment on attachment 159506 [details]
Patch (fix nits).

Thanks.

[WebKit Review Bot]

Comment on attachment 159506 [details]
Patch (fix nits).

Rejecting attachment 159506 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
er-set-timeout-blocked-expected.txt
patching file LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker-eval.js
patching file LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker-function-function.js
patching file LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker-set-timeout.js

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/13616310

[Thomas Sepez]

Created attachment 160811 [details]
Patch, update-webkit

re-upload patch.  Unclear if out of date or if infrastructure itself is borked.

[Adam Barth]

Comment on attachment 160811 [details]
Patch, update-webkit

I'm building this locally and will land manually.

[Adam Barth]

Committed r126947: <http://trac.webkit.org/changeset/126947>