1433005 - (CVE-2018-5130) Crash when WebRTC RTP payload type is incorrect

Status: VERIFIED FIXED

(Core :: WebRTC, defect, P1)

Description

[Tony Paloma]

We discovered by accident that if from our remote WebRTC endpoint we send RTP packets with a correct SSRC and body but an incorrect PayloadType we can crash Firefox and cause invalid read/write or divide by zero exceptions.

The details of what we are sending are RTP packets with contents originally generated by a client using Chrome with opus/48000/2 contents, but the payload type from Chrome is 111 and we were inadvertently putting that into the payload type for the packet forwarded to Firefox rather than 109 as Firefox desired via the negotiated SDP for it.  The Firefox client receiving the packet seemed to consider it valid, about:webrtc showed packets being received and no errors, various WebRTC logging in Firefox also looked happy, but fairly quickly we’d crash with one of the below presumably because the audio decoding was invalid due to incorrect payload type.

Once we fixed our server code forwarding these packets to set correct payload types all the crashes stopped, but this appears to be a remotely exploitable crash bug at the least with possible remote code execution potential if packets were carefully crafted and targeted? So just in case, I've marked this as a security bug.

Links to submitted crash reports:

https://crash-stats.mozilla.com/report/index/bp-c1cd72f1-a6f4-40de-84bb-a1ac30180123 - Write Violation
https://crash-stats.mozilla.com/report/index/bp-ffd44cad-17cc-4d8c-b94d-98b0f0180123 - Divide by Zero
https://crash-stats.mozilla.com/report/index/54087df8-9b05-4c7c-af2e-a2c270180123 - Read Violation

Comment 1

[Al Billings [:abillings - ex-MoCo]]

Randell, can you take a look at this?

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Forwarding...

Comment 3

[Byron Campen [:bwc]]

This is all stuff in the media stack. We could maybe add some filtering logic in MediaPipeline that will strip out bad packets, but the fact that the media stack screws up the memory management when the payload type is wrong suggests that there are probably similar memory management bugs that will be triggered by bad payloads, which is something we can't check in MediaPipeline...

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

We've fuzzed some video stuff, but this is audio data... probably we should fuzz audio inputs (feed in "Opus" data (and other codecs!) with fuzzed data (wrong/wrong-length/right-header-wrong-rest, etc).  Most of these appear to be in NetEq, which is after the codecs; it would be interesting to see what the codecs are decoding to or if failure/sanity (output length) checks are missing somewhere.

Also check upstream for neteq fixes.

Comment 5

[Nils Ohlmeier [:drno]]

Yeah I was also thinking about checking for the PT type. But I don't get why the PT actually matters after the transport layer. So I would like to replicate to find out the root cause, rather then plastering over it.

Comment 6

[Christoph Diehl [:posidron]]

Our current version is:

#define WEBRTC_SVNVERSION 7864
#define WEBRTC_PULL_DATE "Wed Dec 10 12:23:33 EST 2014"

Though, I went back through the commits dating back 4 month and picked out the most obvious things to me:

https://webrtc.googlesource.com/src/+/bc5c69f8e7a5b4b8b9dcfc979d8bff09ffb78daa Use of unititialized value in AECM.
https://webrtc.googlesource.com/src/+/d2b5b1f5bae94c24cc279fe198ff28236821f1ac Division by zero in NoiseSuppression.
https://webrtc.googlesource.com/src/+/ab20a6016c5d0798a00dd566c78f5f49065a9492 AEC-m and AEC-2 fuzzing.
https://webrtc.googlesource.com/src/+/49ccbdb9d69f47b66df286e766f18f95dc611119 Add fuzzer for ForwardErrorCorrection::DecodeFec.
https://webrtc.googlesource.com/src/+/32c6ae249fb879d75de119bc63b9a16f9f14bdca Fix fuzzer-found undefined behavior in webrtc_cng

Before Dec 10:
https://webrtc.googlesource.com/src/+/5dcbbfd1533bf14efee5b6bd2b879d515d1095c5 Create a fuzzer for ComfortNoiseDecoder
https://webrtc.googlesource.com/src/+/bc5a40870c6d83642523d27c660f75896ac5d6c3 Fix off-by-one error when removing information about missing packet in PacketBuffer.
https://webrtc.googlesource.com/src/+/8731176b922ad7d82d0407f2ac36d9f5396f9ac2 NetEq: Fix an UBSan error
https://webrtc.googlesource.com/src/+/ba68aabb06b627e1773fdfdcbeec05cd50c8c500 Fix of integer overflow in WebRtcAecm_ProcessBlock / ApmTest.Process


I'm going to add some SDP injection routines to our WebRTC DOM module. We lately sticked in fuzzing SDP separately.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

(In reply to Christoph Diehl [:posidron] from comment #6)
> Our current version is:
> 
> #define WEBRTC_SVNVERSION 7864
> #define WEBRTC_PULL_DATE "Wed Dec 10 12:23:33 EST 2014"

Actually, we're on Bug 1341285: Webrtc updated to branch 57 rev 52b6562a10b495; initial pull made Feb 2 2017 14:38 EST

They switched off SVN some time ago, so that file is no longer relevant.

> 
> Though, I went back through the commits dating back 4 month and picked out
> the most obvious things to me:
> 
> https://webrtc.googlesource.com/src/+/
> bc5c69f8e7a5b4b8b9dcfc979d8bff09ffb78daa Use of unititialized value in AECM.
> https://webrtc.googlesource.com/src/+/
> d2b5b1f5bae94c24cc279fe198ff28236821f1ac Division by zero in
> NoiseSuppression.
> https://webrtc.googlesource.com/src/+/
> ab20a6016c5d0798a00dd566c78f5f49065a9492 AEC-m and AEC-2 fuzzing.
> https://webrtc.googlesource.com/src/+/
> 49ccbdb9d69f47b66df286e766f18f95dc611119 Add fuzzer for
> ForwardErrorCorrection::DecodeFec.
> https://webrtc.googlesource.com/src/+/
> 32c6ae249fb879d75de119bc63b9a16f9f14bdca Fix fuzzer-found undefined behavior
> in webrtc_cng
> 
> Before Dec 10:
> https://webrtc.googlesource.com/src/+/
> 5dcbbfd1533bf14efee5b6bd2b879d515d1095c5 Create a fuzzer for
> ComfortNoiseDecoder
> https://webrtc.googlesource.com/src/+/
> bc5a40870c6d83642523d27c660f75896ac5d6c3 Fix off-by-one error when removing
> information about missing packet in PacketBuffer.
> https://webrtc.googlesource.com/src/+/
> 8731176b922ad7d82d0407f2ac36d9f5396f9ac2 NetEq: Fix an UBSan error
> https://webrtc.googlesource.com/src/+/
> ba68aabb06b627e1773fdfdcbeec05cd50c8c500 Fix of integer overflow in
> WebRtcAecm_ProcessBlock / ApmTest.Process

Those may still be relevant, however.  We can cherrypick if needed/useful.

Comment 8

[Christoph Diehl [:posidron]]

Thanks jesup, I first overlooked '14 and after posting thought we wouldn't update it for cherry picks or minor updates. "Before Dec 10" are of course simply from the past December.

Comment 9

[Christoph Diehl [:posidron]]

Attachment: testcase.html

Based on the description of the reporter I tried to come up with a testcase (attached):

See line 36:

sdp.replace(/a=rtpmap:\d+ opus\/48000\/2\r\n/g, 'a=rtpmap:111 opus/4800/2\r\n')

Seem to crash reliably.

Comment 10

[Christoph Diehl [:posidron]]

Attachment: callstack.txt

Comment 11

[Christoph Diehl [:posidron]]

Attachment: testcase.html

Comment 12

[Dan Minor [:dminor]]

Ran the supplied testcase (thank you!) on an ASAN build, appears to be memory corruption in JsepTrack:

==23077==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000263e98 at pc 0x7f21358a788f bp 0x7fff0f4a6bf0 sp 0x7fff0f4a6be8
WRITE of size 8 at 0x602000263e98 thread T0 (file:// Content)
    #0 0x7f21358a788e in mozilla::JsepTrack::NegotiateCodecs(mozilla::SdpMediaSection const&, std::vector<mozilla::JsepCodecDescription*, std::allocator<mozilla::JsepCodecDescription*> >*, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >*) const /home/dminor/src/firefox-signaling/media/webrtc/signaling/src/jsep/JsepTrack.cpp:501:28
    #1 0x7f213589743d in mozilla::JsepTrack::Negotiate(mozilla::SdpMediaSection const&, mozilla::SdpMediaSection const&) /home/dminor/src/firefox-signaling/media/webrtc/signaling/src/jsep/JsepTrack.cpp:515:3
    #2 0x7f2135894518 in mozilla::JsepSessionImpl::MakeNegotiatedTransceiver(mozilla::SdpMediaSection const&, mozilla::SdpMediaSection const&, bool, unsigned long, mozilla::JsepTransceiver*) /home/dminor/src/firefox-signaling/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:1069:29
    #3 0x7f2135888c4a in mozilla::JsepSessionImpl::HandleNegotiatedSession(mozilla::UniquePtr<mozilla::Sdp, mozilla::DefaultDelete<mozilla::Sdp> > const&, mozilla::UniquePtr<mozilla::Sdp, mozilla::DefaultDelete<mozilla::Sdp> > const&) /home/dminor/src/firefox-signaling/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:993:10
    #4 0x7f213588d7fc in mozilla::JsepSessionImpl::SetRemoteDescriptionAnswer(mozilla::JsepSdpType, mozilla::UniquePtr<mozilla::Sdp, mozilla::DefaultDelete<mozilla::Sdp> >) /home/dminor/src/firefox-signaling/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:1376:17
    #5 0x7f213588d7fc in mozilla::JsepSessionImpl::SetRemoteDescription(mozilla::JsepSdpType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/dminor/src/firefox-signaling/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:924
    #6 0x7f21359877c5 in mozilla::PeerConnectionImpl::SetRemoteDescription(int, char const*) /home/dminor/src/firefox-signaling/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:1876:32
    #7 0x7f21380f7bf6 in mozilla::PeerConnectionImpl::SetRemoteDescription(int, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) /home/dminor/src/firefox-signaling/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h:368:10
    #8 0x7f21380f7bf6 in mozilla::dom::PeerConnectionImplBinding::setRemoteDescription(JSContext*, JS::Handle<JSObject*>, mozilla::PeerConnectionImpl*, JSJitMethodCallArgs const&) /home/dminor/src/firefox-signaling/objdir-ff-asan/dom/bindings/PeerConnectionImplBinding.cpp:240
    #9 0x7f2139da4f13 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/dminor/src/firefox-signaling/dom/bindings/BindingUtils.cpp:3036:13
    #10 0x7f21411d509d in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /home/dminor/src/firefox-signaling/js/src/jscntxtinlines.h:291:15

Comment 13

[Dan Minor [:dminor]]

Oops, I see the callstack was already attached :)

Comment 14

[Dan Minor [:dminor]]

Attachment: Don't insert DTMF codec twice

The problem was that we only had the DTMF codec present in the codecs array, so we would attempt to insert a second copy of it past the end of the array, and then resize the array to accommodate it. This moves the resize so that we are guaranteed to have enough space for any extra codecs we're adding. It also adds a check that prevents us from adding a second copy of the DTMF codec, as that leads to a use-after-free soon after.

Comment 15

[Byron Campen [:bwc]]

Comment on attachment 8945831 [details] [diff] [review]
Don't insert DTMF codec twice

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

This is getting hard to read. My suggestion is to:

1. Find each entry we want to use (the actual codec, the dtmf, the red, and anything else), and remove it from the codec array.
2. Empty out the codec array, deleting anything that is left.
3. Build a new codec array.

Comment 16

[Dan Minor [:dminor]]

Attachment: Simplify codec pruning in NegotiateCodecs

That makes sense to me.

Comment 17

[Byron Campen [:bwc]]

Comment on attachment 8945843 [details] [diff] [review]
Simplify codec pruning in NegotiateCodecs

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

::: media/webrtc/signaling/src/jsep/JsepTrack.cpp
@@ +492,5 @@
>    if (!codecs->empty() && !red) {
> +    std::vector<JsepCodecDescription*> codecsToKeep;
> +
> +    // Keep our preferred codec.
> +    codecsToKeep.push_back((*codecs)[0]);

Might this be dtmf or ulpfec? We probably need to find by iterating, probably in the same loop that handles deletions below.

@@ +504,2 @@
>      for (size_t i = 1; i < codecs->size(); ++i) {
> +      if (dtmf != (*codecs)[i]) {

Probably need to check ulpfec here too.

Comment 18

[Dan Minor [:dminor]]

(In reply to Byron Campen [:bwc] from comment #17)
> Comment on attachment 8945843 [details] [diff] [review]
> Simplify codec pruning in NegotiateCodecs
> 
> Review of attachment 8945843 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: media/webrtc/signaling/src/jsep/JsepTrack.cpp
> @@ +492,5 @@
> >    if (!codecs->empty() && !red) {
> > +    std::vector<JsepCodecDescription*> codecsToKeep;
> > +
> > +    // Keep our preferred codec.
> > +    codecsToKeep.push_back((*codecs)[0]);
> 
> Might this be dtmf or ulpfec? We probably need to find by iterating,
> probably in the same loop that handles deletions below.
> 

Does this mean that we should iterate through the array and take the first codec that is not dtmf or ulpfec as our preferred codec? Shouldn't the sort done at [1] make sure that the preferred codec is sorted ahead of dtmf or ulpfec? If not, should we fix the sort?

> @@ +504,2 @@
> >      for (size_t i = 1; i < codecs->size(); ++i) {
> > +      if (dtmf != (*codecs)[i]) {
> 
> Probably need to check ulpfec here too.

We are now keeping ulpfec as well as dtmf?

It might be safer to just fix the crash here and make any behavioural changes in a non-sec bug where things are easier to test.

[1] https://searchfox.org/mozilla-central/rev/062e1cf551f5bf3f0af33671b818f75a55ac497b/media/webrtc/signaling/src/jsep/JsepTrack.cpp#483

Comment 19

[Byron Campen [:bwc]]

(In reply to Dan Minor [:dminor] from comment #18)
> (In reply to Byron Campen [:bwc] from comment #17)
> > Comment on attachment 8945843 [details] [diff] [review]
> > Simplify codec pruning in NegotiateCodecs
> > 
> > Review of attachment 8945843 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: media/webrtc/signaling/src/jsep/JsepTrack.cpp
> > @@ +492,5 @@
> > >    if (!codecs->empty() && !red) {
> > > +    std::vector<JsepCodecDescription*> codecsToKeep;
> > > +
> > > +    // Keep our preferred codec.
> > > +    codecsToKeep.push_back((*codecs)[0]);
> > 
> > Might this be dtmf or ulpfec? We probably need to find by iterating,
> > probably in the same loop that handles deletions below.
> > 
> 
> Does this mean that we should iterate through the array and take the first
> codec that is not dtmf or ulpfec as our preferred codec? Shouldn't the sort
> done at [1] make sure that the preferred codec is sorted ahead of dtmf or
> ulpfec? If not, should we fix the sort?

  That sort only causes "strongly preferred" codecs to override the preference indicated by the remote end. Unless prefs are set, there are no "strongly preferred" codecs. (https://searchfox.org/mozilla-central/rev/062e1cf551f5bf3f0af33671b818f75a55ac497b/media/webrtc/signaling/src/jsep/JsepTrack.cpp#388) So, you'll need to iterate.

> > @@ +504,2 @@
> > >      for (size_t i = 1; i < codecs->size(); ++i) {
> > > +      if (dtmf != (*codecs)[i]) {
> > 
> > Probably need to check ulpfec here too.
> 
> We are now keeping ulpfec as well as dtmf?
> 
> It might be safer to just fix the crash here and make any behavioural
> changes in a non-sec bug where things are easier to test.
> 
> [1]
> https://searchfox.org/mozilla-central/rev/
> 062e1cf551f5bf3f0af33671b818f75a55ac497b/media/webrtc/signaling/src/jsep/
> JsepTrack.cpp#483

It is easier to reason about this code's memory safety if we cover all the bases.

Comment 20

[Dan Minor [:dminor]]

Thank you for the explanation, I was having a hard time reconciling the changes you were asking for with the current behaviour.

Comment 21

[Dan Minor [:dminor]]

Attachment: Simplify codec pruning in NegotiateCodecs

Comment 22

[Byron Campen [:bwc]]

Comment on attachment 8945891 [details] [diff] [review]
Simplify codec pruning in NegotiateCodecs

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

::: media/webrtc/signaling/src/jsep/JsepTrack.cpp
@@ +496,5 @@
> +        codecsToKeep.push_back(codec);
> +      } else if (codec == ulpfec) {
> +        codecsToKeep.push_back(codec);
> +      } else if (!foundPreferredCodec) {
> +        codecsToKeep.push_back(codec);

I wonder if webrtc.org is gonna barf if this isn't first? Should check this, or put it at the front using insert.

Comment 23

[Dan Minor [:dminor]]

Attachment: Simplify codec pruning in NegotiateCodecs

This inserts the preferred codec at the beginning of the vector, as suggested. Carrying forward r+.

Comment 24

[Dan Minor [:dminor]]

Comment on attachment 8946238 [details] [diff] [review]
Simplify codec pruning in NegotiateCodecs

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Since the patch looks like a refactor, I don't think the underlying problem it fixes is that obvious.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No.
Which older supported branches are affected by this flaw?
All, the code was introduced in Bug 1291714.
If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I believe the same patch will apply to all branches. If not, the changes required should be minimal.
How likely is this patch to cause regressions; how much testing does it need?
Unlikely, I would expect our standard unit tests to catch any problems.

Comment 25

[Al Billings [:abillings - ex-MoCo]]

sec-approval+ for trunk. Can we get the patch nominated for Beta and ESR52 as well (though we might delay ESR52 checkin for a week or two to not shine a giant spotlight on it as a security bug).

Comment 26

[Dan Minor [:dminor]]

Comment on attachment 8946238 [details] [diff] [review]
Simplify codec pruning in NegotiateCodecs

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1291714
[User impact if declined]:
Crashes / security problems.
[Is this code covered by automated tests?]:
Not currently, but there is a testcase attached to this bug that could be worked into mochitest.
[Has the fix been verified in Nightly?]:
No.
[Needs manual test from QE? If yes, steps to reproduce]:
No. 
[List of other uplifts needed for the feature/fix]:
None.
[Is the change risky?]:
No.
[Why is the change risky/not risky?]:
This is just a small change to how codecs are chosen during negotiation.
[String changes made/needed]:
None.

Comment 27

[Dan Minor [:dminor]]

Comment on attachment 8946238 [details] [diff] [review]
Simplify codec pruning in NegotiateCodecs

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Crashes / security problems.
Fix Landed on Version:
Will land on 60.
Risk to taking this patch (and alternatives if risky): 
Limited risk.
String or UUID changes made by this patch: 
None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 28

[Dan Minor [:dminor]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/62da02a0aa77e00dc89de31040bb6fbc357b5136

Comment 29

[Dan Minor [:dminor]]

Attachment: Fix up JsepSessionTest::ValidateAnsweredCodecParams gtest

I forgot to run the gtests before landing this. We're now keeping the ulpfec codec which breaks some of the tests in JsepSessionTest::ValidateAnsweredCodecParams. This looks like expected changes to me, but I wanted to double check just to be safe. Assuming this is fine, I'll roll this up into the main patch when I reland.

Comment 30

[Byron Campen [:bwc]]

Comment on attachment 8946708 [details] [diff] [review]
Fix up JsepSessionTest::ValidateAnsweredCodecParams gtest

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

Verify with jesup that enabling ulpfec is something we want to do. If not, we need to comment those lines out, and put a TODO + bug number in the source.

::: media/webrtc/signaling/gtest/jsep_session_unittest.cpp
@@ +3565,4 @@
>    // ASSERT_EQ(3U, video_section.GetFormats().size());
>    ASSERT_EQ("120", video_section.GetFormats()[0]);
> +  ASSERT_EQ("123", video_section.GetFormats()[1]); // ULPFEC
> +  //ASSERT_EQ("121", video_section.GetFormats()[1]);

Re-index these I guess.

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for Gtest failures on Linux x64 asan and Windows 10 x64 debug:

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc0e8ad85292559a7099016daa4ec25a0f70f630

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=62da02a0aa77e00dc89de31040bb6fbc357b5136&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=159356534&repo=mozilla-inbound

[task 2018-01-30T16:17:56.476Z] 16:17:56  WARNING -  TEST-UNEXPECTED-FAIL | JsepSessionTest.ValidateAnsweredCodecParams |       Expected: 1U
[task 2018-01-30T16:17:56.476Z] 16:17:56     INFO -        Which is: 1
[task 2018-01-30T16:17:56.477Z] 16:17:56     INFO -  To be equal to: video_section.GetFormats().size()
[task 2018-01-30T16:17:56.477Z] 16:17:56     INFO -        Which is: 2 @ /builds/worker/workspace/build/src/media/webrtc/signaling/gtest/jsep_session_unittest.cpp:3564
[task 2018-01-30T16:17:56.478Z] 16:17:56  WARNING -  TEST-UNEXPECTED-FAIL | JsepSessionTest.ValidateAnsweredCodecParams | test completed (time: 7ms)

Comment 32

[Dan Minor [:dminor]]

Randell, I'm guessing we shouldn't retain the ulpfec codec here until we enable ulpfec support over in Bug 875922?

Comment 33

[Randell Jesup [:jesup] (needinfo me)]

Correct - and certainly wouldn't block this fix for something we're not enabling yet

Comment 34

[Dan Minor [:dminor]]

Attachment: Simplify codec pruning in NegotiateCodecs (no ulpfec codec)

Version of the patch with ulpfec disabled.

Comment 35

[Dan Minor [:dminor]]

Attachment: Simplify codec pruning in NegotiateCodecs (no ulpfec codec)

Correct version of patch :/

Comment 36

[Dan Minor [:dminor]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/97c9705b1be2dc26f2d434986c614d49ff1f4549

Comment 37

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/97c9705b1be2

Comment 38

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8946789 [details] [diff] [review]
Simplify codec pruning in NegotiateCodecs (no ulpfec codec)

Crash fix for a sec-high issue, let's uplift it. (This patch rather than the originally marked one)

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/90ae99509f1e

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/a6a9e26688c1

Comment 41

[Ciprian Georgiu, Desktop QA]

I have reproduced this bug using an affected ASAN build, 59.01 Nightly (2018-01-15) and running the test case from comment 11.

I cannot reproduce it anymore on the latest ASAN builds under Ubuntu 16.04 x64: latest Nightly 60.0a1 (2018-03-05), Beta 59.0b14 (20180303190036) and 52.7.0 esr (20180304210040).