Bug 1428684 - reduce the chance of UAF when changing states of MDSM.
SetState() will delete the current state object and UAF will happen if members
are accessed after this call. However, sometimes it is not obvious if SetState()
is called indirectly as we do in MaybeFinishSeek().
To make it less error-prone, we will keep the old state object alive for a bit
longer and set mMaster to null to catch potential UAF.
MozReview-Commit-ID: Aqxj92ETjti
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -285,29 +285,36 @@ protected:
// |aArgs| must be passed by reference to avoid passing MOZ_NON_PARAM class
// SeekJob by value. See bug 1287006 and bug 1338374. But we still *must*
// copy the parameters, because |Exit()| can modify them. See bug 1312321.
// So we 1) pass the parameters by reference, but then 2) immediately copy
// them into a Tuple to be safe against modification, and finally 3) move
// the elements of the Tuple into the final function call.
auto copiedArgs = MakeTuple(Forward<Ts>(aArgs)...);
- // keep mMaster in a local object because mMaster will become invalid after
- // the current state object is deleted.
+ // Copy mMaster which will reset to null.
auto master = mMaster;
auto* s = new S(master);
MOZ_ASSERT(GetState() != s->GetState() ||
GetState() == DECODER_STATE_SEEKING);
SLOG("change state to: %s", ToStateStr(s->GetState()));
Exit();
+ // Delete the old state asynchronously to avoid UAF if the caller tries to
+ // access its members after SetState() returns.
+ master->OwnerThread()->DispatchDirectTask(
+ NS_NewRunnableFunction("MDSM::StateObject::DeleteOldState",
+ [toDelete = Move(master->mStateObj)](){}));
+ // Also reset mMaster to catch potentail UAF.
+ mMaster = nullptr;
+
master->mStateObj.reset(s);
return CallEnterMemberFunction(s, copiedArgs,
typename IndexSequenceFor<Ts...>::Type());
}
RefPtr<MediaDecoder::SeekPromise>
SetSeekingState(SeekJob&& aSeekJob, EventVisibility aVisibility);