1650112 - "script.callFunctionOn" and "script.evaluate" fail evaluating "new Function()" on pages with CSP

Status: NEW

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

Description

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

Running the following Puppeteer code snippet fails on CSP enabled web pages:

  await page.goto("https://people.mozilla.org")
  await page.evaluate(() => {
    var a = new Function();
  });
  await browser.close()

The failure as reported is:

Error: Evaluation failed: call to Function() blocked by CSP
    at ExecutionContext._evaluateInternal (/Users/henrik/code/gecko/remote/test/puppeteer/lib/ExecutionContext.js:102:19)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async ExecutionContext.evaluate (/Users/henrik/code/gecko/remote/test/puppeteer/lib/ExecutionContext.js:33:16)

As such both the `Runtime.callFunctionOn` and `Runtime.evaluate` APIs are affected.

Given that Puppeteer uses a lot of evaluation we should get this into our beta reserve list.

Comment 1

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

Note that this works in Marionette:

1593700450886	Marionette	DEBUG	3 -> [0,2,"WebDriver:Navigate",{"url":"https://people.mozilla.org"}]
1593700450890	Marionette	TRACE	[5] Received DOM event beforeunload for about:blank
1593700451593	Marionette	TRACE	[5] Received DOM event pagehide for about:blank
1593700452637	Marionette	TRACE	[5] Received DOM event DOMContentLoaded for https://people.mozilla.org/
1593700453530	Marionette	TRACE	[5] Received DOM event pageshow for https://people.mozilla.org/
1593700453532	Marionette	DEBUG	3 <- [1,2,null,{"value":null}]
1593700453541	Marionette	DEBUG	3 -> [0,3,"WebDriver:ExecuteScript",{"script":"new Function()","newSandbox":true,"args":[],"filename":"_a/test_minimized.py","sandbox":"default","line":51}]

In the Remote agent we call this._debuggee.executeInGlobal(expression) without creating a sandbox first. Maybe that is the problem here?

DevTools is using dbgWindow.executeInGlobalWithBindings() here.

Alexandre, do you have some info for us how devtools work with script evaluation on pages with CSP enabled?

Comment 2

[Alexandre Poirot [:ochameau]]

I don't know exactly about JS evaluations, but Logan may know.

Comment 3

[Logan Smyth [:loganfsmyth]]

From what I can see, CSP restrictions are entirely realm-based, so you can never directly use eval or new Function directly in the page content without it running through CSP validation. Given that, the options would seem to be

• Run the code in another realm that can interact with the content realm (which I think is what marrionette does with sandboxes)
• Run the code via the debugger's executeInGlobal directly (and never use eval or Function, directly or indirectly)
• Add support in SpiderMonkey to mark a given source as allowed to skip CSP checks, which is vaguely similar to the mutedErrors feature SpiderMonkey has to mark sources loaded from remote origins. executeInGlobal could then be expanded to allow setting this flag.

I imagine this is also an issue for the FF webconsole, does Chrome's console allow executing eval() and new Function in the console on pages that have CSP protection enabled?

Comment 4

[James Graham [:jgraham]]

Pretty sure the right thing to do here is just copy what marionette does.

Comment 5

[Logan Smyth [:loganfsmyth]]

The main question I have about Marionette's approach is how close it needs to be to acting exactly like a script running in the actual page. It look's like Marionette uses a sandbox that has the original page global as a prototype, so globals set in the content will mostly work, but I'd guess that something like globalThis.a = 4 would assign the property on the sandbox global, but it would not be visible to code running in the page. At least that's my guess. It's as close as you could get to running in the content without running in the content, but there are likely ways that the code will behave that are slightly different, which could be surprising to users.

The test-case I'd be curious about is an HTML file with <script>var a = "a"; function getA(){ return a; }</script> with marionette evalling a = "b"; console.log(getA()); does that log "a" or "b"? I think the assignment will assign a global a on the sandbox, but not change the global in the actual page, so getA() would still return "a", which I'd expect to be surprising for users.

Comment 6

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

(In reply to Logan Smyth [:loganfsmyth] from comment #5)

The test-case I'd be curious about is an HTML file with <script>var a = "a"; function getA(){ return a; }</script> with marionette evalling a = "b"; console.log(getA()); does that log "a" or "b"? I think the assignment will assign a global a on the sandbox, but not change the global in the actual page, so getA() would still return "a", which I'd expect to be surprising for users.

Indeed. When using the system (no?) sandbox and not creating a new one the value is still a:

1595232648234	Marionette	DEBUG	3 -> [0,2,"WebDriver:Navigate",{"url":"data:text/html;charset=utf-8,<script>var a = 'a'; function getA(){ return a; }</script>"}]
1595232648239	Marionette	TRACE	[10] Received DOM event beforeunload for about:blank
1595232648243	Marionette	TRACE	[10] Received DOM event pagehide for about:blank
1595232648250	Marionette	TRACE	[10] Received DOM event DOMContentLoaded for data:text/html;charset=utf-8,<script>var a = 'a'; function getA(){ return a; }</script>
1595232648264	Marionette	TRACE	[10] Received DOM event pageshow for data:text/html;charset=utf-8,<script>var a = 'a'; function getA(){ return a; }</script>
1595232648268	Marionette	DEBUG	3 <- [1,2,null,{"value":null}]
1595232648285	Marionette	DEBUG	3 -> [0,3,"WebDriver:ExecuteScript",{"script":"a = 'b'; return getA();","newSandbox":true,"args":[],"filename":"_a/test_minimized.py","sandbox":null,"line":55}]
1595232648287	Marionette	DEBUG	3 <- [1,3,null,{"value":"a"}]

James, I won't have the time this week to dig into the script execution stuff this week. Maybe you can have a look how we actually should behave and if scripts as evaluated should affect the page's state or just the sandbox? I assume this all will also influence the spec work around the BiDi protocol. Thanks.

Comment 7

[James Graham [:jgraham]]

I actually ran into this; it seems we can't share globals between the injected script and the page, as predicted. I'm pretty sure the spec doesn't expect this and it's kind of surprising we don't get bug reports.

Comment 8

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

(In reply to James Graham [:jgraham] from comment #7)

I actually ran into this; it seems we can't share globals between the injected script and the page, as predicted. I'm pretty sure the spec doesn't expect this and it's kind of surprising we don't get bug reports.

So it sounds like a bug in the WebDriver spec. Would you mind filing a new issue for that?

Comment 9

[James Graham [:jgraham]]

I don't think other browsers have the equivalent of our sandboxes, so they might disagree with the assessment that it's a bug in the spec. It seems more reasonable to consider it a bug in marionette.

Comment 10

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

Getting CSP to work correctly when we will start implementing the script execution for WebDriver BiDi:
https://github.com/w3c/webdriver-bidi/issues/63

Until then we won't have the time to do any work on this particular bug.

Comment 11

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

We are not going to implement these arguments for CDP but will have it eventually for WebDriver BiDi.

Comment 12

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

Sorry, I've accidentally closed this bug by end of last week. It was not intended.

We actually have the same problem with our WebDriver BiDi implementation and should still take care of it. Instead of filing a new bug lets just move this bug from CDP to BiDi.