1428684 - Reduce the chance of UAF when changing states of MDSM

Status: RESOLVED FIXED

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

Description

[JW Wang [:jwwang] [:jw_wang]]

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.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 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.

Review commit: https://reviewboard.mozilla.org/r/210800/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210800/

Comment 2

[Tzuhao Kuo [: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?

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

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.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Thanks!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/afb1b6f44c25
reduce the chance of UAF when changing states of MDSM. r=kaku

Comment 6

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/afb1b6f44c25