Closed Bug 1290966 Opened 4 years ago Closed 4 years ago

Permission denied to access properties when callback argument is applied during script evaluation

Categories

(Testing :: Marionette, defect)

50 Branch
defect
Not set

Tracking

(firefox49 fixed, firefox50 fixed, firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: titus.fortner, Assigned: ato)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.82 Safari/537.36

Steps to reproduce:

Using the javascript here: https://github.com/watir/watir-webdriver/blob/master/lib/watir-webdriver/atoms/fireEvent.js
in the fire_event_function variable below:

-> POST session/0cfca641-3d1c-1048-9c6d-18a4a7331715/execute/sync
   >>> http://127.0.0.1:4445/session/0cfca641-3d1c-1048-9c6d-18a4a7331715/execute/sync | {"script":"return (/#{fire_event_function).apply(null, arguments)","args":[{"ELEMENT":"544ab0b2-d8a4-0545-87b2-3d50ffa321e9","element-6066-11e4-a52e-4f735466cecf":"544ab0b2-d8a4-0545-87b2-3d50ffa321e9"},"focus"]}
     > {"Accept"=>"application/json", "Content-Type"=>"application/json; charset=utf-8", "Content-Length"=>"9483"}



Actual results:

<- {"error":"javascript error","message":"Error: Permission denied to access property \"bubble\""}


Expected results:

javascript should execute without error
Flags: needinfo?(ato)
Andreas can you have a look at this please
return (/#{fire_event_function).apply looks like invalid JavaScript to me.
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Flags: needinfo?(ato)
Resolution: --- → INVALID
Sorry, #{fire_event_function}" was supposed to be the placeholder variable for all the javascript here: 
https://github.com/watir/watir-webdriver/blob/master/lib/watir-webdriver/atoms/fireEvent.js

Sometimes I make things more confusing when I'm trying to make them easier.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
I don’t understand why the script you linked to uses .apply(null, arguments).  This is done implicitly by WebDriver.  Moreover, I can’t get this to work because of a syntax error:

JavascriptException: JavascriptException: SyntaxError: function statement requires a name
stacktrace:
	execute_script @test_fire_event.py, line 48
	inline javascript, line 0
	src: "__webDriverArguments.push(__webDriverCallback);(function() { return // Copyright 2011 Software Freedom Conservatory"
Okay, I was able to reproduce the problem you describe by removing the comment from the script.  Does the script exist in a non-minified version?  Or better, do you have a minimised test case?  This will require _a lot_ of effort on my part to figure out.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(titus.fortner)
I think I found the problem.  In https://github.com/mozilla/gecko-dev/blob/master/testing/marionette/evaluate.js#L115 we are pushing the resolve callback onto the arguments applied to the injected function in the same statement that the script is wrapped in.

The following appears to fix the problem:

diff --git a/testing/marionette/evaluate.js b/testing/marionette/evaluate.js
index 2fbeb44..c4e4705 100644
--- a/testing/marionette/evaluate.js
+++ b/testing/marionette/evaluate.js
@@ -111,9 +111,9 @@ evaluate.sandbox = function(sb, script, args = [], opts = {}) {
     if (!opts.directInject) {
       sb[CALLBACK] = sb[COMPLETE];
       sb[ARGUMENTS] = Cu.cloneInto(args, sb, {wrapReflectors: true});
+      sb[ARGUMENTS].push(CALLBACK);
 
-      script = `${ARGUMENTS}.push(${CALLBACK});` +
-          `(function() { ${script} }).apply(null, ${ARGUMENTS})`;
+      script = `(function() { ${script} }).apply(null, ${ARGUMENTS})`;
 
       // marionetteScriptFinished is not WebDriver conformant,
       // hence it is only exposed to immutable sandboxes
Flags: needinfo?(titus.fortner)
Assignee: nobody → ato
Status: NEW → ASSIGNED
Blocks: webdriver
Summary: execute script permission denied → Permission denied to access properties when callback argument is applied during script evaluation
The callback argument that is pushed onto the arguments array applied to
the injected function, needs to be safely cloned.  Previously we pushed
it to the array at the time of evaluation (Cu.evalInSandbox).

Review commit: https://reviewboard.mozilla.org/r/68572/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68572/
Attachment #8776939 - Flags: review?(dburns)
Attachment #8776939 - Flags: review?(dburns)
Review commit: https://reviewboard.mozilla.org/r/69306/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69306/
Attachment #8776939 - Attachment description: Bug 1290966 - Safely push callback argument before script evaluation; → Bug 1290966 - Permit introspection into arguments;
Attachment #8777891 - Flags: review?(dburns)
Attachment #8776939 - Flags: review?(dburns)
Comment on attachment 8776939 [details]
Bug 1290966 - Permit introspection into arguments;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68572/diff/1-2/
I fixed the wrong problem in my previous patch, where I mistakenly removed the callback argument for asynchronous scripts altogether.  Instead I pushed the stringified name of the function onto `arguments`.  Oops.

The core of the problem is that the callback function to return from asynchronous scripts is defined in the privileged content frame script and later pushed on to the arguments object in the scope of the sandboxed script.  When the script tries to look at any property related to this object, it will yield a “permission denied” error.

I’ve now submitted a patch to allow this, which “hides” the real callback function inside a dummy wrapper defined in the sandbox.
Titus, can you try out one of the builds from https://treeherder.mozilla.org/#/jobs?repo=try&revision=af3874b31c7feddfafd9555ad165e124490fe3e8 and see if they resolve your original issue?
Flags: needinfo?(titus.fortner)
Comment on attachment 8776939 [details]
Bug 1290966 - Permit introspection into arguments;

https://reviewboard.mozilla.org/r/68572/#review66858
Attachment #8776939 - Flags: review?(dburns) → review+
Comment on attachment 8777891 [details]
Bug 1290966 - Remove callback function argument from sync scripts;

https://reviewboard.mozilla.org/r/69306/#review66860
Attachment #8777891 - Flags: review?(dburns) → review+
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f5c531ba572
Permit introspection into arguments; r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/ca2b685228c1
Remove callback function argument from sync scripts; r=automatedtester
(In reply to Andreas Tolfsen ‹:ato› from comment #11)
> Titus, can you try out one of the builds from
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=af3874b31c7feddfafd9555ad165e124490fe3e8 and see if
> they resolve your original issue?

I tried downloading a dmg from: http://archive.mozilla.org/pub/firefox/try-builds/atolfsen@mozilla.com-af3874b31c7feddfafd9555ad165e124490fe3e8/try-macosx64/

and still getting: 

```
JavaScriptError: [POST http://localhost:4444/wd/hub/session/1b6320fe-4c49-4e03-80dd-3887b8560502/execute / {"script":"return (function () {\n        try {\n          localStorage.clear();\n          sessionStorage.clear();\n        } catch (e) {\n          console.log('Failed to clearBrowserState');\n          // if cookies are disabled, this will blow up some browsers.\n        }\n        return true;\n      }).apply(this, arguments);","args":[]}] Permission denied to access property "__raven_wrapper__" (WARNING: The server did not provide any stacktrace information)
Command duration or timeout: 12 milliseconds
```

Name 	Firefox
Version 	51.0a1
Build ID 	20160804103347
Flags: needinfo?(ato)
https://hg.mozilla.org/mozilla-central/rev/5f5c531ba572
https://hg.mozilla.org/mozilla-central/rev/ca2b685228c1
Status: ASSIGNED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
(In reply to Vlad Filippov :vladikoff from comment #15)
> I tried downloading a dmg from:
> http://archive.mozilla.org/pub/firefox/try-builds/atolfsen@mozilla.com-
> af3874b31c7feddfafd9555ad165e124490fe3e8/try-macosx64/
> 
> and still getting: 
> 
> ```
> JavaScriptError: [POST
> http://localhost:4444/wd/hub/session/1b6320fe-4c49-4e03-80dd-3887b8560502/
> execute / {"script":"return (function () {\n        try {\n         
> localStorage.clear();\n          sessionStorage.clear();\n        } catch
> (e) {\n          console.log('Failed to clearBrowserState');\n          //
> if cookies are disabled, this will blow up some browsers.\n        }\n      
> return true;\n      }).apply(this, arguments);","args":[]}] Permission
> denied to access property "__raven_wrapper__" (WARNING: The server did not
> provide any stacktrace information)
> Command duration or timeout: 12 milliseconds
> ```

I’m still not able to reproduce this, can you create a reproducible test?

I also have some questions about the above:

- What is __raven_wrapper__ and where is it referenced in the above script?

- The script inside the wrapper function uses try…catch which means it can never fail.  Which line is causing the error?  Can you please include the response from the server?

- If I uncomment the try…catch encapsulation, I get a SecurityError: The operation is insecure on calling localStorage.clear().  I think this is a bug, but probably not the same as you’re reporting.
Flags: needinfo?(ato) → needinfo?(vlad)
(In reply to Andreas Tolfsen ‹:ato› from comment #17)
> (In reply to Vlad Filippov :vladikoff from comment #15)
> > I tried downloading a dmg from:
> > http://archive.mozilla.org/pub/firefox/try-builds/atolfsen@mozilla.com-
> > af3874b31c7feddfafd9555ad165e124490fe3e8/try-macosx64/
> > 
> > and still getting: 
> > 
> > ```
> > JavaScriptError: [POST
> > http://localhost:4444/wd/hub/session/1b6320fe-4c49-4e03-80dd-3887b8560502/
> > execute / {"script":"return (function () {\n        try {\n         
> > localStorage.clear();\n          sessionStorage.clear();\n        } catch
> > (e) {\n          console.log('Failed to clearBrowserState');\n          //
> > if cookies are disabled, this will blow up some browsers.\n        }\n      
> > return true;\n      }).apply(this, arguments);","args":[]}] Permission
> > denied to access property "__raven_wrapper__" (WARNING: The server did not
> > provide any stacktrace information)
> > Command duration or timeout: 12 milliseconds
> > ```
> 
> I’m still not able to reproduce this, can you create a reproducible test?
> 
> I also have some questions about the above:
> 
> - What is __raven_wrapper__ and where is it referenced in the above script?
> 
> - The script inside the wrapper function uses try…catch which means it can
> never fail.  Which line is causing the error?  Can you please include the
> response from the server?
> 
> - If I uncomment the try…catch encapsulation, I get a SecurityError: The
> operation is insecure on calling localStorage.clear().  I think this is a
> bug, but probably not the same as you’re reporting.

Here's the only information I have so far:

* Existing issue that was filed against raven-js (that introduces a __raven_wrapper) and mentions Selenium: https://github.com/getsentry/raven-js/issues/495
* `wrap` function that introduces it: https://github.com/getsentry/raven-js/blob/d9c9d85e4c0981d06b775ba37a70c9271796533d/src/raven.js#L208
Flags: needinfo?(vlad)
Are you able to come up with a self-contained, minimised test case?
Flags: needinfo?(vlad)
s/minimised/minimal/
(In reply to Andreas Tolfsen ‹:ato› from comment #19)
> Are you able to come up with a self-contained, minimised test case?

Here you go: https://github.com/vladikoff/bz1290966

## STR

* clone the repo, run Selenum 3 beta2, `npm install` repo
* run `npm test`

You should get this in Firefox 48:

```
× firefox on any platform - storage - clear (0.647s)
JavaScriptError: [POST http://localhost:4444/wd/hub/session/5f75bdb3-0260-458f-9cc8-14a140b966a7/execute / {"script":"return (function () {\n          try {\n            localStorage.clear();\n            sessionStorage.clear();\n          } catch (e) {\n            console.log('Failed to clearBrowserState');\n            // if cookies are disabled, this will blow up some browsers.\n          }\n          return true;\n        }).apply(this, arguments);","args":[]}] Permission denied to access property "__raven_wrapper__" (WARNING: The server did not provide any stacktrace information)
Command duration or timeout: 9 milliseconds
Build info: version: '3.0.0-beta2', revision: '2aa21c1', time: '2016-08-02 15:03:28 -0700'
System info: host: 'vladikoff-2', ip: '192.168.1.174', os.name: 'Mac OS X', os.arch: 'x86_64', os.version: '10.11.6', java.version: '1.8.0_92'
Driver info: org.openqa.selenium.firefox.MarionetteDriver
Capabilities [{rotatable=false, raisesAccessibilityExceptions=false, marionette=true, appBuildId=20160726073904, version=48.0, platform=MAC, proxy={}, command_id=1, specificationLevel=0, acceptSslCerts=false, browserVersion=48.0, platformVersion=15.6.0, name=intern.js, XULappId={ec8030f7-c20a-464f-9b0e-13a3a9e97384}, browserName=Firefox, takesScreenshot=true, idle-timeout=60, takesElementScreenshot=true, platformName=Darwin, device=desktop}]
Session ID: cbc3e00d-7117-6746-b6af-2d3ed4e4eae3
  at runRequest  <node_modules/intern/node_modules/leadfoot/Session.js:88:40>
  at <node_modules/intern/node_modules/leadfoot/Session.js:109:39>
  at new Promise  <node_modules/intern/node_modules/dojo/Promise.ts:411:3>
  at ProxiedSession._post  <node_modules/intern/node_modules/leadfoot/Session.js:63:10>
  at ProxiedSession.Session.execute  <node_modules/intern/node_modules/leadfoot/Session.js:643:21>
  at Command.<anonymous>  <node_modules/intern/node_modules/leadfoot/Command.js:635:19>
  at <node_modules/intern/node_modules/dojo/Promise.ts:393:15>
  at run  <node_modules/intern/node_modules/dojo/Promise.ts:237:7>
  at <node_modules/intern/node_modules/dojo/nextTick.ts:44:3>
  at nextTickCallbackWith0Args  <node.js:420:9>
  at Command.target.(anonymous function) [as execute]  <node_modules/intern/node_modules/leadfoot/Command.js:610:11>
  at Test.registerSuite.clear [as test]  <test.js:15:10>
  at <node_modules/intern/lib/Test.js:181:24>
  at <node_modules/intern/browser_modules/dojo/Promise.ts:393:15>
  at runCallbacks  <node_modules/intern/browser_modules/dojo/Promise.ts:11:11>
  at <node_modules/intern/browser_modules/dojo/Promise.ts:317:4>
  at run  <node_modules/intern/browser_modules/dojo/Promise.ts:237:7>
  at <node_modules/intern/browser_modules/dojo/nextTick.ts:44:3>
  at nextTickCallbackWith0Args  <node.js:420:9>
  at process._tickCallback  <node.js:349:13>
No unit test coverage for firefox on any platform
firefox on any platform: 1/1 tests failed
```

You can modify the setting here: https://github.com/vladikoff/bz1290966/blob/master/intern.js#L8 to point to a different version of Firefox. (If Firefox gets stuck on a blank screen when the test starts just put a single character into the url bar and press enter to trigger a Google search result, then the test will keep running.)
Flags: needinfo?(vlad)
Flags: needinfo?(ato)
Sheriffs: Test-only changes.
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
Blocks: 1280947
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
Anything that involves npm and downloading an armada of dependencies is unfortunately not very self-contained.
Flags: needinfo?(titus.fortner)
Flags: needinfo?(ato)
You need to log in before you can comment on or make changes to this bug.