Bug 1540378 - Web Authentication: Fix teardown during cycle collection r=keeler,mccr8

In Bug 1448408 ("Don't listen to visibility events"), it became possible to
close a tab without a visibility event to cause transactions to cancel. This
is a longstanding bug that was covered up by the visibility events. This patch
updates the cycle collection code to ensure that transactions get cleared out
safely, and we don't proceed to RejectTransaction (and subsequent code) on
already-cycle-collected objects.

Differential Revision: https://phabricator.services.mozilla.com/D25641

--HG--
extra : moz-landing-system : lando
This commit is contained in:
J.C. Jones 2019-04-01 23:13:26 +00:00
parent 9060a01dd4
commit bfdf3e2380
4 changed files with 30 additions and 10 deletions

View File

@ -51,8 +51,16 @@ NS_INTERFACE_MAP_END_INHERITING(WebAuthnManagerBase)
NS_IMPL_ADDREF_INHERITED(U2F, WebAuthnManagerBase)
NS_IMPL_RELEASE_INHERITED(U2F, WebAuthnManagerBase)
NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_INHERITED(U2F, WebAuthnManagerBase,
mTransaction)
NS_IMPL_CYCLE_COLLECTION_CLASS(U2F)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(U2F, WebAuthnManagerBase)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mTransaction)
NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
tmp->ClearTransaction();
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(U2F, WebAuthnManagerBase)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTransaction)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(U2F)
/***********************************************************************
* Utility Functions
@ -513,7 +521,7 @@ void U2F::FinishGetAssertion(const uint64_t& aTransactionId,
}
void U2F::ClearTransaction() {
if (!NS_WARN_IF(mTransaction.isNothing())) {
if (!mTransaction.isNothing()) {
StopListeningForVisibilityEvents();
}

View File

@ -141,11 +141,12 @@ class U2F final : public WebAuthnManagerBase, public nsWrapperCache {
MOZ_CAN_RUN_SCRIPT void ExecuteCallback(T& aResp,
nsMainThreadPtrHandle<C>& aCb);
// Clears all information we have about the current transaction.
void ClearTransaction();
// Rejects the current transaction and clears it.
MOZ_CAN_RUN_SCRIPT void RejectTransaction(const nsresult& aError);
// Clears all information we have about the current transaction.
void ClearTransaction();
nsString mOrigin;
// The current transaction, if any.

View File

@ -38,8 +38,18 @@ static mozilla::LazyLogModule gWebAuthnManagerLog("webauthnmanager");
NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(WebAuthnManager,
WebAuthnManagerBase)
NS_IMPL_CYCLE_COLLECTION_INHERITED(WebAuthnManager, WebAuthnManagerBase,
mFollowingSignal, mTransaction)
NS_IMPL_CYCLE_COLLECTION_CLASS(WebAuthnManager)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(WebAuthnManager,
WebAuthnManagerBase)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mFollowingSignal)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mTransaction)
tmp->ClearTransaction();
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(WebAuthnManager,
WebAuthnManagerBase)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFollowingSignal)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTransaction)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
/***********************************************************************
* Utility Functions
@ -156,7 +166,7 @@ nsresult RelaxSameOrigin(nsPIDOMWindowInner* aParent,
**********************************************************************/
void WebAuthnManager::ClearTransaction() {
if (!NS_WARN_IF(mTransaction.isNothing())) {
if (!mTransaction.isNothing()) {
StopListeningForVisibilityEvents();
}

View File

@ -116,11 +116,12 @@ class WebAuthnManager final : public WebAuthnManagerBase, public AbortFollower {
private:
virtual ~WebAuthnManager();
// Clears all information we have about the current transaction.
void ClearTransaction();
// Rejects the current transaction and calls ClearTransaction().
void RejectTransaction(const nsresult& aError);
// Clears all information we have about the current transaction.
void ClearTransaction();
// The current transaction, if any.
Maybe<WebAuthnTransaction> mTransaction;
};