bug 1483905 - ensure the WebAuthnManager stays alive while WebAuthnTransactionChild is using it r=qdot
authorDana Keeler <dkeeler@mozilla.com>
Fri, 07 Sep 2018 09:17:19 -0700
changeset 494729 88ebd0e4c45e3759dbc7aac01b6e1848be254f10
parent 494728 31863507b67034637f43ea3a75db362cf3538b85
child 494730 a7dc4f0c4710d4a3f671f1d6bd9ef464f51f33ba
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1483905
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1483905 - ensure the WebAuthnManager stays alive while WebAuthnTransactionChild is using it r=qdot Differential Revision: https://phabricator.services.mozilla.com/D5305
dom/webauthn/WebAuthnTransactionChild.cpp
dom/webauthn/tests/get_assertion_dead_object.html
dom/webauthn/tests/mochitest.ini
dom/webauthn/tests/test_webauthn_get_assertion_dead_object.html
--- a/dom/webauthn/WebAuthnTransactionChild.cpp
+++ b/dom/webauthn/WebAuthnTransactionChild.cpp
@@ -23,41 +23,56 @@ WebAuthnTransactionChild::WebAuthnTransa
 mozilla::ipc::IPCResult
 WebAuthnTransactionChild::RecvConfirmRegister(const uint64_t& aTransactionId,
                                               const WebAuthnMakeCredentialResult& aResult)
 {
   if (NS_WARN_IF(!mManager)) {
     return IPC_FAIL_NO_REASON(this);
   }
 
-  mManager->FinishMakeCredential(aTransactionId, aResult);
+  // We don't own the reference to mManager. We need to prevent its refcount
+  // going to 0 while we call anything that can reach the call to
+  // StopListeningForVisibilityEvents in WebAuthnManager::ClearTransaction
+  // (often via WebAuthnManager::RejectTransaction).
+  RefPtr<WebAuthnManagerBase> kungFuDeathGrip(mManager);
+  kungFuDeathGrip->FinishMakeCredential(aTransactionId, aResult);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 WebAuthnTransactionChild::RecvConfirmSign(const uint64_t& aTransactionId,
                                           const WebAuthnGetAssertionResult& aResult)
 {
   if (NS_WARN_IF(!mManager)) {
     return IPC_FAIL_NO_REASON(this);
   }
 
-  mManager->FinishGetAssertion(aTransactionId, aResult);
+  // We don't own the reference to mManager. We need to prevent its refcount
+  // going to 0 while we call anything that can reach the call to
+  // StopListeningForVisibilityEvents in WebAuthnManager::ClearTransaction
+  // (often via WebAuthnManager::RejectTransaction).
+  RefPtr<WebAuthnManagerBase> kungFuDeathGrip(mManager);
+  kungFuDeathGrip->FinishGetAssertion(aTransactionId, aResult);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 WebAuthnTransactionChild::RecvAbort(const uint64_t& aTransactionId,
                                     const nsresult& aError)
 {
   if (NS_WARN_IF(!mManager)) {
     return IPC_FAIL_NO_REASON(this);
   }
 
-  mManager->RequestAborted(aTransactionId, aError);
+  // We don't own the reference to mManager. We need to prevent its refcount
+  // going to 0 while we call anything that can reach the call to
+  // StopListeningForVisibilityEvents in WebAuthnManager::ClearTransaction
+  // (often via WebAuthnManager::RejectTransaction).
+  RefPtr<WebAuthnManagerBase> kungFuDeathGrip(mManager);
+  kungFuDeathGrip->RequestAborted(aTransactionId, aError);
   return IPC_OK();
 }
 
 void
 WebAuthnTransactionChild::ActorDestroy(ActorDestroyReason why)
 {
   // Called by either a __delete__ message from the parent, or when the
   // channel disconnects. Clear out the child actor reference to be sure.
new file mode 100644
--- /dev/null
+++ b/dom/webauthn/tests/get_assertion_dead_object.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset=utf-8>
+</head>
+<body>
+<script type="text/javascript">
+  window.addEventListener('load', function() {
+    let o = [];
+    o[0] = window.navigator;
+    document.writeln('');
+    // Since the USB token is enabled by default, this will pop up a notification that the
+    // user can insert/interact with it. Since this is just a test, this won't happen. The
+    // request will eventually time out.
+    // Unfortunately the minimum timeout is 15 seconds.
+    o[0].credentials.get({ publicKey: { challenge: new Uint8Array(128), timeout: 15000 } });
+    o.forEach((n, i) => o[i] = null);
+  });
+</script>
+</body>
+</html>
--- a/dom/webauthn/tests/mochitest.ini
+++ b/dom/webauthn/tests/mochitest.ini
@@ -1,20 +1,22 @@
 [DEFAULT]
 support-files =
   cbor.js
   u2futil.js
   pkijs/*
+  get_assertion_dead_object.html
 skip-if = !e10s
 scheme = https
 
 [test_webauthn_abort_signal.html]
 [test_webauthn_attestation_conveyance.html]
 [test_webauthn_authenticator_selection.html]
 [test_webauthn_authenticator_transports.html]
 [test_webauthn_loopback.html]
 [test_webauthn_no_token.html]
 [test_webauthn_make_credential.html]
 [test_webauthn_get_assertion.html]
+[test_webauthn_get_assertion_dead_object.html]
 [test_webauthn_override_request.html]
 [test_webauthn_store_credential.html]
 [test_webauthn_sameorigin.html]
 [test_webauthn_isplatformauthenticatoravailable.html]
new file mode 100644
--- /dev/null
+++ b/dom/webauthn/tests/test_webauthn_get_assertion_dead_object.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<head>
+  <title>Test for GetAssertion on dead object</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+
+  <h1>Test for GetAssertion on dead object</h1>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1483905">Mozilla Bug 1483905</a>
+
+  <script class="testbody" type="text/javascript">
+    "use strict";
+    SimpleTest.waitForExplicitFinish();
+    SimpleTest.requestFlakyTimeout(
+      "Due to the nature of this test, there's no way for the window we're opening to signal " +
+      "that it's done (the `document.writeln('')` is essential and basically clears any state " +
+      "we could use). So, we have to wait at least 15 seconds for the webauthn call to time out.");
+    let win = window.open("https://example.com/tests/dom/webauthn/tests/get_assertion_dead_object.html");
+    setTimeout(() => {
+      win.close();
+      SimpleTest.finish();
+    }, 20000);
+  </script>
+
+</body>
+</html>