Closed Bug 1428684 Opened 2 years ago Closed 2 years ago

Reduce the chance of UAF when changing states of MDSM

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

Details

Attachments

(1 file)

https://searchfox.org/mozilla-central/rev/f42618c99dcb522fb674221acfbc68c2d92e7936/dom/media/MediaDecoderStateMachine.cpp#279-282

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

https://searchfox.org/mozilla-central/rev/f42618c99dcb522fb674221acfbc68c2d92e7936/dom/media/MediaDecoderStateMachine.cpp#1310

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.
Attachment #8940591 - Flags: review?(kaku)
Comment on attachment 8940591 [details]
Bug 1428684 - reduce the chance of UAF when changing states of MDSM.

https://reviewboard.mozilla.org/r/210800/#review217016

Isn't the original way easier to show a pattern of some potential faults?
Attachment #8940591 - Flags: review?(kaku) → review+
Comment on attachment 8940591 [details]
Bug 1428684 - reduce the chance of UAF when changing states of MDSM.

https://reviewboard.mozilla.org/r/210800/#review217016

Not really. The original code will result in UAF which might show a crash stack which seems unrelated to MDSM.
Thanks!
Assignee: nobody → jwwang
Priority: -- → P3
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/afb1b6f44c25
reduce the chance of UAF when changing states of MDSM. r=kaku
https://hg.mozilla.org/mozilla-central/rev/afb1b6f44c25
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.