Skip to content

Commit

Permalink
Bug 1353074 - Make unload event safe for introspection from content; …
Browse files Browse the repository at this point in the history
…r=maja_zf

Marionette does not protect the unloadHandler in
testing/marionette/evaluate.js from content introspection or
modification, which can happen when web frameworks override
window.addEventListener/window.removeEventListener.

The script evaluation module used in Marionette relies on
sandbox.window.addEventListener/removeEventListener to throw an error when
script execution is aborted due to the document unloading itself.  If the
window.addEventListener/removeEventListener functions have been overridden
to introspect the objects that are passed, they may inadvertently touch
objects originating from chrome space, such as the unloadHandler.

Because the Gecko sandboxing system put in place strict security measures
to prevent accidental chrome-space modification from content, inspecting
the unloadHandler will throw a permission denied error once the script
has finished executing.

We have found examples in the wild of this in particular with the Angular
web framework.  This patch makes the unloadHandler safe for introspection
from web content.

Fixes: mozilla/geckodriver#515
MozReview-Commit-ID: E2LgPhLLuDT

--HG--
extra : rebase_source : 9948585b4ac2f464a9f31868bfd2d5967e61755e
  • Loading branch information
andreastt committed Apr 3, 2017
1 parent cd1f0d6 commit 42acd45
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
7 changes: 4 additions & 3 deletions testing/marionette/evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ evaluate.sandbox = function (sb, script, args = [], opts = {}) {
let src = "";
sb[COMPLETE] = resolve;
timeoutHandler = () => reject(new ScriptTimeoutError("Timed out"));
unloadHandler = () => reject(
new JavaScriptError("Document was unloaded during execution"));
unloadHandler = sandbox.cloneInto(
() => reject(new JavaScriptError("Document was unloaded during execution")),
sb);

// wrap in function
if (!opts.directInject) {
Expand Down Expand Up @@ -145,7 +146,7 @@ evaluate.sandbox = function (sb, script, args = [], opts = {}) {

// timeout and unload handlers
scriptTimeoutID = setTimeout(timeoutHandler, opts.timeout || DEFAULT_TIMEOUT);
sb.window.onunload = sandbox.cloneInto(unloadHandler, sb);
sb.window.onunload = unloadHandler;

let res;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,18 +263,40 @@ def content_timeout_triggered(mn):
content_timeout_triggered,
message="Scheduled setTimeout event was cancelled by call to execute_script")

def test_privileged_code_inspection(self):
# test permission denied on toString of unload event handler
def test_access_chrome_objects_in_event_listeners(self):
# sandbox.window.addEventListener/removeEventListener
# is used by Marionette for installing the unloadHandler which
# is used to return an error when a document is unloaded during
# script execution.
#
# Certain web frameworks, notably Angular, override
# window.addEventListener/removeEventListener and introspects
# objects passed to them. If these objects originates from chrome
# without having been cloned, a permission denied error is thrown
# as part of the security precautions put in place by the sandbox.

# addEventListener is called when script is injected
self.marionette.navigate(inline("""
<script>
window.addEventListener = (type, handler) => handler.toString();
</script>"""))
window.addEventListener = (event, listener) => listener.toString();
</script>
"""))
self.marionette.execute_script("", sandbox=None)

# removeEventListener is called when sandbox is unloaded
self.marionette.navigate(inline("""
<script>
window.removeEventListener = (event, listener) => listener.toString();
</script>
"""))
self.marionette.execute_script("", sandbox=None)

def test_access_global_objects_from_chrome(self):
# test inspection of arguments
self.marionette.execute_script("__webDriverArguments.toString()")



class TestExecuteChrome(WindowManagerMixin, TestExecuteContent):

def setUp(self):
Expand Down Expand Up @@ -333,6 +355,9 @@ def test_window_set_timeout_is_not_cancelled(self):
def test_privileged_code_inspection(self):
pass

def test_access_chrome_objects_in_event_listeners(self):
pass


class TestElementCollections(MarionetteTestCase):

Expand Down

0 comments on commit 42acd45

Please sign in to comment.