1941780 - script.evaluate and script.callFunction should bypass CSP

Status: NEW

(Remote Protocol :: WebDriver BiDi, defect, P2)

Description

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

STRs:

• start webdriver bidi session
• open tab on page with CSP preventing eval (eg Content-Security-Policy: script-src 'none';)
• use script.evaluate with expression = eval(1+1)

ER:
Should return 2
AR:
CSP exception is thrown

Comment 1

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Attachment: test_page_csp_none.html

Comment 2

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Points might be wrong, hard to know how complicated this will be on the platform side.

Comment 3

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Note that we have the same issue in DevTools at the moment: Bug 1580514

Comment 4

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Probably a question for SpiderMonkey at this point.

Arai, hi! Do you know if eval/evalWithBindings can be used to evaluate code which bypasses CSPs defined by the content page?

Comment 5

[Tooru Fujisawa [:arai]]

I have 3 questions in order to understand the situation:

• (a) Is this specific to eval() in the code?
• (b) Can you provide more details around how to reproduce it, or the exact error and some more details?
• (c) What's the relation between script.evaluate and Debugger's eval/evalWithBindings right now?

For (b), is it "EvalError: call to eval() blocked by CSP" error?
If that's the case, it's the error thrown by EvalKernel, and the callback used there is nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction.
So, if there's some special handling needed, either of the following will be a solution:

• if the specific situation is already represented in the nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction's parameter, handle it accordingly there
• otherwise, somehow add a parameter to the related codepath, and handle it in nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction or perhaps in EvalKernal.

For (a) and (c), is script.evaluate already using the Debugger API's eval/evalWithBindings? If so, it shouldn't be using EvalKernel directly, unless there's eval() call inside the script. So I would assume this issue is specific to eval() being inside the code?

Comment 6

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Thanks Arai!

(a) Is this specific to eval() in the code?

Sorry I should have given more context. So WebDriver BiDi provides two commands to automation clients to evaluate JS in a content page: script.evaluate and script.callFunction. They slightly differ in terms of semantics for the end user, but otherwise the underlying implementation is very similar. And it's also very similar to evaluating code with the DevTools' Webconsole.

Those commands use either executeInGlobal or executeInGlobalWithBindings (and not eval/evalWithBindings, not sure why I had those names in mind) on the debugger global (created via makeGlobalObjectReference).

The use case here is that a client uses script.evaluate to execute JS which contains eval or new Function. And if the page is defining a CSP preventing this, calling executeInGlobal with code containing eval/new Function will indeed throw with "EvalError: call to eval() blocked by CSP". So yes, it's not an issue overall when using script.evaluate on a page with CSPs, it's only triggered if the code we try to run contains eval/new Function.

I could provide a BiDi test case, but the easiest way to reproduce the same issue is with the DevTools webconsole:

• open https://csp-devtools.glitch.me/
• open webconsole
• try to execute eval("window")

Does that change something for the suggestions you provided already?

Comment 7

[Tooru Fujisawa [:arai]]

Thank you for the details!
It makes sense now.

And I think the suggested way would be the same as above. Modify either the CSP's hook or the eval handling, and somehow use the information about Debugger API being used, and bypass the CSP restriction in that situation.

If the information is not yet directly there (that's what I can see for now), the possible options would be something like the following:

• use the enclosing frame information (the caller parameter below) to determine if it's inside Debugger API's execution
  • this would work if we can somehow mark the outer-most frame as Debugger API's execution. but this won't work in promise reactions etc, given the frame won't retain the initial one
• add a new flag about the Debugger API execution, either only in SpiderMonkey side or both in SpiderMonkey and Gecko side, where the flag is propagated through promises in the same way as JS::PromiseUserInputEventHandlingState, and refer it from the CSP handling to by pass the check

We should be careful not to leak the "bypass the CSP check" mode to webpage's code, and the promise handling part might be too risky unless there's demand for that usage.

https://searchfox.org/mozilla-central/rev/345ec3c55ddda5f0ce37168f0644dcdcc4834409/js/src/builtin/Eval.cpp#233-235

static bool EvalKernel(JSContext* cx, HandleValue v, EvalType evalType,
                       AbstractFramePtr caller, HandleObject env,
                       jsbytecode* pc, MutableHandleValue vp) {

I'll see if there's some other solution, or some flag or execution mode (if any) which can be used or used as a reference.

Comment 9

[Tom Schuster (MoCo)]

WebExtension content scripts are also exempted from the page CSP, to achieve this they currently use ChromeUtils.compileScript and script.executeInGlobal. It would be good to share some code here, so that it's easier to audit the places that might bypass CSPs.

Comment 10

[Simon Friedberger [:simonf]]

Fixing this bug will let us enforce a CSP for the files in chrome://remote/content/marionette/. These are currently exempt: https://searchfox.org/mozilla-central/source/dom/security/nsContentSecurityUtils.cpp.

This is important because we can disable the other testing exceptions outside of tests but the marionette tests don't have the automation flag xpc::IsInAutomation() set.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Just a quick note: a fix here won’t immediately resolve issues with chrome://remote/content/marionette/ files, but it would demonstrate how we could address it for Marionette as well. An alternative approach would be to move those files into an extension, as suggested in bug 1344267, rather than bundling them with the Firefox release build.

Nevertheless WebDriver BiDi will likely face a similar situation soon, once we support evaluating scripts in the parent process, we’ll also need a way to serve test files in that context.

Lets discuss this bug in our next triage meeting.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We are going to try to get bug 1344267 fixed so that we are not shipping these files.

But nevertheless it would be good to know how to reproduce the problem with those chrome URLs. Simon, do you have some steps that we could run locally to see those CSP assertions? Thanks.

Comment 13

[Simon Friedberger [:simonf]]

Add a CSP to the test files and it will fail. Here is the patch I would like to apply to enforce a CSP: https://phabricator.services.mozilla.com/D247045

Here is an example try run: https://treeherder.mozilla.org/jobs?repo=try&revision=2f0a0f8b2ea89d9efeb00d676bc8469eb507dcd4 have a look at "unit".

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks, Simon. One last question though:

If we were to package those XHTML files within an add-on, similar to how Mochitest does, would we still need to apply the CSP changes to them?

Also does it mean we’re enforcing CSP even in the chrome scope? It most likely means that we would also fail with script evaluation for certain JS code unless this bug is fixed, right?

Comment 15

[Simon Friedberger [:simonf]]

We are working on enforcing CSPs in all chrome and about pages. We're almost finished though, so if it isn't failing yet, maybe you don't have a problem.

Comment 16

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Fix should be straightforward, let's table it for m18.