Closed Bug 1148572 Opened 6 years ago Closed 6 years ago

Re-enabling a H264 m-line results in UAF crash

Categories

(Core :: WebRTC: Audio/Video, defect)

39 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- unaffected
firefox38 --- fixed
firefox39 --- fixed
firefox40 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- fixed
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- unaffected
b2g-master --- fixed

People

(Reporter: drno, Assigned: drno)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-uaf, sec-high)

Attachments

(1 file)

When a previously disabled H264 m-section gets re-enabled through renegotiation we currently create a new encoder here:
https://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp#856

Which then results in releasing the previous encoder object here:
https://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp#1039

And ultimately the video engine trying to use the freed memory.

READ of size 8 at 0x612000432f48 thread T0
    #0 0x7f0621e25b18 in DeleteEncoder /home/nohlmeier/src/mozilla-central/media/webrtc/trunk/webrtc/modules/video_coding/main/source/generic_encoder.cc:91
    #1 0x7f0621e7c7cf in RegisterSendCodec /home/nohlmeier/src/mozilla-central/media/webrtc/trunk/webrtc/modules/video_coding/main/source/video_sender.cc:127
    #2 0x7f0621d3eb36 in SetEncoder /home/nohlmeier/src/mozilla-central/media/webrtc/trunk/webrtc/video_engine/vie_encoder.cc:403
    #3 0x7f0621d3cf28 in SetSendCodec /home/nohlmeier/src/mozilla-central/media/webrtc/trunk/webrtc/video_engine/vie_codec_impl.cc:214
    #4 0x7f061ca5db39 in ConfigureSendMediaCodec /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:633
    #5 0x7f061ca91048 in GetOrCreateVideoConduit /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp:770
    #6 0x7f061ca8c974 in CreateOrUpdateMediaPipeline /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp:387
    #7 0x7f061caf62a7 in UpdateMediaPipelines /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp:384
    #8 0x7f061cac8dd8 in SetSignalingState_m /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2438
    #9 0x7f061cabd284 in UpdateSignalingState /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2501
    #10 0x7f061e0de129 in SetRemoteDescription /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h:377
    #11 0x7f061f54190a in GenericBindingMethod /home/nohlmeier/src/mozilla-central/dom/bindings/BindingUtils.cpp:2492
    #12 0x7f0623fe2f51 in CallJSNative /home/nohlmeier/src/mozilla-central/js/src/jscntxtinlines.h:235
    #13 0x7f062401d9cf in Interpret /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:2617
    #14 0x7f0624002d48 in RunScript /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:452
    #15 0x7f0623fe345f in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:521
    #16 0x7f0623fa0be8 in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:558
    #17 0x7f0624ac7192 in Call /home/nohlmeier/src/mozilla-central/js/src/jsapi.cpp:4343
    #18 0x7f061e1c5124 in Call /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/bindings/./PromiseBinding.cpp:47
    #19 0x7f06206fcbf8 in Call /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/promise/../../dist/include/mozilla/dom/PromiseBinding.h:72
    #20 0x7f0620702b8e in Constructor /home/nohlmeier/src/mozilla-central/dom/promise/Promise.cpp:584
    #21 0x7f061e1e2f22 in _constructor /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/bindings/./PromiseBinding.cpp:576
    #22 0x7f061c6bdac8 in construct /home/nohlmeier/src/mozilla-central/js/xpconnect/wrappers/XrayWrapper.cpp:1649
    #23 0x7f0624d28183 in construct /home/nohlmeier/src/mozilla-central/js/src/proxy/Proxy.cpp:410
    #24 0x7f0624d2b412 in proxy_Construct /home/nohlmeier/src/mozilla-central/js/src/proxy/Proxy.cpp:712
    #25 0x7f062402e8ab in CallJSNativeConstructor /home/nohlmeier/src/mozilla-central/js/src/jscntxtinlines.h:235
    #26 0x7f062401d9be in Interpret /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:2614
    #27 0x7f0624002d48 in RunScript /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:452
    #28 0x7f0623fe345f in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:521
    #29 0x7f0623fa0be8 in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:558
    #30 0x7f06244f37bf in DoCallFallback /home/nohlmeier/src/mozilla-central/js/src/jit/BaselineIC.cpp:9640
    #31 0x7f060590c62f  (<unknown module>)

0x612000432f48 is located 8 bytes inside of 280-byte region [0x612000432f40,0x612000433058)
freed by thread T0 here:
    #0 0x4ad910 in __interceptor_free _asan_rtl_
    #1 0x7f061ca62978 in assign /home/nohlmeier/src/mozilla-central/objdir-ff-asan/media/webrtc/signaling/signaling_ecc/../../../../dist/include/nsAutoPtr.h:41 (discriminator 3)
    #2 0x7f061ca9657e in EnsureExternalCodec /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp:859
    #3 0x7f061ca90ec5 in GetOrCreateVideoConduit /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp:765
    #4 0x7f061ca8c974 in CreateOrUpdateMediaPipeline /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp:387
    #5 0x7f061caf62a7 in UpdateMediaPipelines /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp:384
    #6 0x7f061cac8dd8 in SetSignalingState_m /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2438
    #7 0x7f061cabd284 in UpdateSignalingState /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2501
    #8 0x7f061e0de129 in SetRemoteDescription /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h:377
    #9 0x7f061f54190a in GenericBindingMethod /home/nohlmeier/src/mozilla-central/dom/bindings/BindingUtils.cpp:2492
    #10 0x7f0623fe2f51 in CallJSNative /home/nohlmeier/src/mozilla-central/js/src/jscntxtinlines.h:235
    #11 0x7f062401d9cf in Interpret /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:2617
    #12 0x7f0624002d48 in RunScript /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:452
    #13 0x7f0623fe345f in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:521
    #14 0x7f0623fa0be8 in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:558
    #15 0x7f0624ac7192 in Call /home/nohlmeier/src/mozilla-central/js/src/jsapi.cpp:4343
    #16 0x7f061e1c5124 in Call /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/bindings/./PromiseBinding.cpp:47
    #17 0x7f06206fcbf8 in Call /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/promise/../../dist/include/mozilla/dom/PromiseBinding.h:72
    #18 0x7f0620702b8e in Constructor /home/nohlmeier/src/mozilla-central/dom/promise/Promise.cpp:584
    #19 0x7f061e1e2f22 in _constructor /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/bindings/./PromiseBinding.cpp:576
    #20 0x7f061c6bdac8 in construct /home/nohlmeier/src/mozilla-central/js/xpconnect/wrappers/XrayWrapper.cpp:1649
    #21 0x7f0624d28183 in construct /home/nohlmeier/src/mozilla-central/js/src/proxy/Proxy.cpp:410
    #22 0x7f0624d2b412 in proxy_Construct /home/nohlmeier/src/mozilla-central/js/src/proxy/Proxy.cpp:712
    #23 0x7f062402e8ab in CallJSNativeConstructor /home/nohlmeier/src/mozilla-central/js/src/jscntxtinlines.h:235
    #24 0x7f062401d9be in Interpret /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:2614
    #25 0x7f0624002d48 in RunScript /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:452
    #26 0x7f0623fe345f in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:521
    #27 0x7f0623fa0be8 in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:558
    #28 0x7f06244f37bf in DoCallFallback /home/nohlmeier/src/mozilla-central/js/src/jit/BaselineIC.cpp:9640
    #29 0x7f060590c62f  (<unknown module>)

previously allocated by thread T0 here:
    #0 0x4adbf0 in __interceptor_malloc _asan_rtl_
    #1 0x4d3c2d in moz_xmalloc /home/nohlmeier/src/mozilla-central/memory/mozalloc/mozalloc.cpp:89
    #2 0x7f061cb18d20 in operator new /home/nohlmeier/src/mozilla-central/objdir-ff-asan/media/webrtc/signaling/signaling_ecc/../../../../dist/include/mozilla/mozalloc.h:196
    #3 0x7f061ca963d6 in EnsureExternalCodec /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp:856
    #4 0x7f061ca90ec5 in GetOrCreateVideoConduit /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp:765
    #5 0x7f061ca8c974 in CreateOrUpdateMediaPipeline /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp:387
    #6 0x7f061caf62a7 in UpdateMediaPipelines /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp:384
    #7 0x7f061cac8dd8 in SetSignalingState_m /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2438
    #8 0x7f061cabd284 in UpdateSignalingState /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2501
    #9 0x7f061e0de129 in SetRemoteDescription /home/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h:377
    #10 0x7f061f54190a in GenericBindingMethod /home/nohlmeier/src/mozilla-central/dom/bindings/BindingUtils.cpp:2492
    #11 0x7f0623fe2f51 in CallJSNative /home/nohlmeier/src/mozilla-central/js/src/jscntxtinlines.h:235
    #12 0x7f062401d9cf in Interpret /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:2617
    #13 0x7f0624002d48 in RunScript /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:452
    #14 0x7f0623fe345f in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:521
    #15 0x7f0623fa0be8 in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:558
    #16 0x7f0624ac7192 in Call /home/nohlmeier/src/mozilla-central/js/src/jsapi.cpp:4343
    #17 0x7f061e1c5124 in Call /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/bindings/./PromiseBinding.cpp:47
    #18 0x7f06206fcbf8 in Call /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/promise/../../dist/include/mozilla/dom/PromiseBinding.h:72
    #19 0x7f0620702b8e in Constructor /home/nohlmeier/src/mozilla-central/dom/promise/Promise.cpp:584
    #20 0x7f061e1e2f22 in _constructor /home/nohlmeier/src/mozilla-central/objdir-ff-asan/dom/bindings/./PromiseBinding.cpp:576
    #21 0x7f061c6bdac8 in construct /home/nohlmeier/src/mozilla-central/js/xpconnect/wrappers/XrayWrapper.cpp:1649
    #22 0x7f0624d28183 in construct /home/nohlmeier/src/mozilla-central/js/src/proxy/Proxy.cpp:410
    #23 0x7f0624d2b412 in proxy_Construct /home/nohlmeier/src/mozilla-central/js/src/proxy/Proxy.cpp:712
    #24 0x7f062402e8ab in CallJSNativeConstructor /home/nohlmeier/src/mozilla-central/js/src/jscntxtinlines.h:235
    #25 0x7f062401d9be in Interpret /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:2614
    #26 0x7f0624002d48 in RunScript /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:452
    #27 0x7f0623fe345f in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:521
    #28 0x7f0623fa0be8 in Invoke /home/nohlmeier/src/mozilla-central/js/src/vm/Interpreter.cpp:558
    #29 0x7f06244f37bf in DoCallFallback /home/nohlmeier/src/mozilla-central/js/src/jit/BaselineIC.cpp:9640

Shadow bytes around the buggy address:
  0x0c248007e590: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c248007e5a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c248007e5b0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c248007e5c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c248007e5d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa
=>0x0c248007e5e0: fa fa fa fa fa fa fa fa fd[fd]fd fd fd fd fd fd
  0x0c248007e5f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c248007e600: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
  0x0c248007e610: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c248007e620: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c248007e630: 00 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==5327==ABORTING
Attachment #8584760 - Flags: review?(rjesup)
Comment on attachment 8584760 [details] [diff] [review]
media_pipeline.patch

I verified that the patch fixes the problem. The idea was to fix the issue with as little new code as possible to get it in before the uplift. The cleaner solution would probably be to add functionality to check if the desired external codec is set already.
Comment on attachment 8584760 [details] [diff] [review]
media_pipeline.patch

Review of attachment 8584760 [details] [diff] [review]:
-----------------------------------------------------------------

r+, but I'd like to see the "correct" patch and get it in/uplifted
Attachment #8584760 - Flags: review?(rjesup) → review+
I think this only got exposed through bug 1144432, which means it only can get abused in 39. I don't see any way how this should get abused before that. So I'm not convinced that we need to fix 38.
(In reply to Nils Ohlmeier [:drno] from comment #4)
> I think this only got exposed through bug 1144432, which means it only can
> get abused in 39. I don't see any way how this should get abused before
> that. So I'm not convinced that we need to fix 38.

I bet you could bypass that bug by putting an mid in the disabled m-section. Also, I think you could also trigger this by going a=inactive and back (the other side being the answerer).
bwc: fair enough.

I'm planing on opening another bug for the cleaner solution after this landed, because then we don't need a sec bug any longer.

Randel: can I land this in central and aurora right now, or do I need more approvals?
You can land it, but you need sec-approval and aurora approval (which should be no problem).  For example, we may want to land them simultaneously.  Ask for both at once.
Comment on attachment 8584760 [details] [diff] [review]
media_pipeline.patch

Approval Request Comment
[Feature/regressing bug #]: The actual code which landed in bug 1091242, but bug 1144432 makes this easily accessible.

[User impact if declined]: With bug 1144432 landed even a legit use case for a WebRTC call which uses the external OpenH264 plugin can easily get into a user-after-free situation and result in a crash. 

[Describe test coverage new/current, TreeHerder]: No specific new test case has been added for this use case. Manual testing during patch development on an ASAN build shows no problems any more with the patch applied.

[Risks and why]: As the patch only uses existing code to check if the plugin ID is set or not, the risk should be pretty small (assuming the plugin ID gets properly set). 

[String/UUID change made/needed]: N/A

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

If the person understands in which cases the old code got invoked the actual exploit is pretty trivial for Fx 39. The exploit does not require any remote side, only the OpenH264 plugin needs to installed and activate (which is default). 

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Not in the patch itself. The backtrace in the Bugzilla this ticket plus comment give a pretty good idea.

Which older supported branches are affected by this flaw?

At least 38. The actual flawed code in MediaPipelineFactory.cpp exists even in 37 and 36.

If not all supported branches, which bug introduced the flaw?

Bug 1144432 makes the abuse of the existing code trivial. Before that probably with more effort the same exploit could be achieved.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Backporting is trivial.

How likely is this patch to cause regressions; how much testing does it need?

As the patch only adds a safe guard by using existing code I think the risk for regression is very small. Manual testing has been done.
Attachment #8584760 - Flags: sec-approval?
Attachment #8584760 - Flags: approval-mozilla-aurora?
Randell: why do you think 37 and 36 are not affected?

The actual code exists even in release:
http://hg.mozilla.org/releases/mozilla-release/file/2ae2d59d99fd/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp#l691

I guess the question is if that code can be abused in 37 and 36?

Randell, Byron: thought on 37 and 36?
Flags: needinfo?(rjesup)
Flags: needinfo?(docfaraday)
I would guess that it would not be possible to hit this bug without renegotiation.
Flags: needinfo?(docfaraday)
(In reply to Byron Campen [:bwc] from comment #10)
> I would guess that it would not be possible to hit this bug without
> renegotiation.

Exactly, you can't re-enable a line without renegotiation.
Flags: needinfo?(rjesup)
Comment on attachment 8584760 [details] [diff] [review]
media_pipeline.patch

Approvals given.
Attachment #8584760 - Flags: sec-approval?
Attachment #8584760 - Flags: sec-approval+
Attachment #8584760 - Flags: approval-mozilla-aurora?
Attachment #8584760 - Flags: approval-mozilla-aurora+
Comment on attachment 8584760 [details] [diff] [review]
media_pipeline.patch

As we just missed the train...

Approval Request Comment
[Feature/regressing bug #]: The actual code which landed in bug 1091242, but bug 1144432 makes this easily accessible.

[User impact if declined]: With bug 1144432 landed even a legit use case for a WebRTC call which uses the external OpenH264 plugin can easily get into a user-after-free situation and result in a crash. 

[Describe test coverage new/current, TreeHerder]: No specific new test case has been added for this use case. Manual testing during patch development on an ASAN build shows no problems any more with the patch applied.

[Risks and why]: As the patch only uses existing code to check if the plugin ID is set or not, the risk should be pretty small (assuming the plugin ID gets properly set). 

[String/UUID change made/needed]: N/A
Attachment #8584760 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/664ecb8172dc
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8584760 [details] [diff] [review]
media_pipeline.patch

Should be in 38 beta 2
Attachment #8584760 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1151134
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.