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

RESOLVED FIXED in Firefox 49

Status

Testing
Marionette
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Titus Fortner, Assigned: ato)

Tracking

(Blocks: 1 bug)

50 Branch
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed, firefox50 fixed, firefox51 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a year ago
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
(Assignee)

Comment 2

a year ago
return (/#{fire_event_function).apply looks like invalid JavaScript to me.
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(ato)
Resolution: --- → INVALID
(Reporter)

Comment 3

a year ago
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 → ---
(Assignee)

Comment 4

a year ago
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"
(Assignee)

Comment 5

a year ago
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)
(Assignee)

Comment 6

a year ago
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)

Updated

a year ago
Assignee: nobody → ato
Status: NEW → ASSIGNED
(Assignee)

Updated

a year ago
Blocks: 721859
(Assignee)

Updated

a year ago
Summary: execute script permission denied → Permission denied to access properties when callback argument is applied during script evaluation
(Assignee)

Comment 7

a year ago
Created attachment 8776939 [details]
Bug 1290966 - Permit introspection into arguments;

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)
(Assignee)

Updated

a year ago
Attachment #8776939 - Flags: review?(dburns)
(Assignee)

Comment 8

a year ago
Created attachment 8777891 [details]
Bug 1290966 - Remove callback function argument from sync scripts;

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 #8776939 - Flags: review?(dburns)
Attachment #8777891 - Flags: review?(dburns)
(Assignee)

Comment 9

a year ago
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/
(Assignee)

Comment 10

a year ago
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.
(Assignee)

Comment 11

a year ago
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+

Comment 14

a year ago
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)

Comment 16

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5f5c531ba572
https://hg.mozilla.org/mozilla-central/rev/ca2b685228c1
Status: ASSIGNED → RESOLVED
Last Resolved: a year agoa year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
(Assignee)

Comment 17

a year ago
(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)
(Assignee)

Comment 19

a year ago
Are you able to come up with a self-contained, minimised test case?
Flags: needinfo?(vlad)
(Assignee)

Comment 20

a year ago
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)
(Assignee)

Comment 22

a year ago
Sheriffs: Test-only changes.
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
(Assignee)

Updated

a year ago
Blocks: 1280947

Comment 23

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/86446c9b103b
https://hg.mozilla.org/releases/mozilla-aurora/rev/a89e3dd6ead8
status-firefox50: --- → fixed

Comment 24

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/c8587071af34
https://hg.mozilla.org/releases/mozilla-beta/rev/40813e35914a
status-firefox49: --- → fixed

Comment 25

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-release/rev/c8587071af34
https://hg.mozilla.org/releases/mozilla-release/rev/40813e35914a
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
(Assignee)

Comment 26

a year ago
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.