Closed Bug 1789908 Opened 2 years ago Closed 2 years ago

Crash in [@ mozilla::SdpFmtpAttributeList::Fmtp::operator== ]

Categories

(Core :: WebRTC: Signaling, defect)

71 Branch
defect

Tracking

()

RESOLVED FIXED
106 Branch
Tracking Status
firefox-esr91 --- wontfix
firefox-esr102 --- wontfix
firefox104 --- wontfix
firefox105 --- wontfix
firefox106 --- fixed

People

(Reporter: jib, Assigned: bwc)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Found this nullptr offset crash researching if we're impacted by bug 1778130. Not a sec-bug perhaps, but the repro steps are identical so I decided to mark it that way for now to reduce visibility.

In short, I shaped the unit-tests in Chrome's CVE fix referenced in bug 1778130 comment 7 into a fiddle, as I wanted to double-check that Chrome's fixes in high-level code we don't share, wasn't protecting vulnerable lower-level libwebrtc code we do share, and found this crash in our high-level equivalent. Once we fix this crash, we should check again.

STRs:

  1. Open https://jsfiddle.net/jib1/8zqa73um/11/

Expected results: No crash
Actual results: bp-c9b9df46-06f9-4db6-a8a3-c1d5e0220908

Regression range:

6:01.47 INFO: Got as far as we can go bisecting nightlies...
6:01.47 INFO: Last good revision: c9c63f8447027d14ec2c541e82aae1014f08f812 (2019-10-02)
6:01.47 INFO: First bad revision: 2e1bfb7458de41a36432671c405eee62d897a761 (2019-10-03)
6:01.47 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c9c63f8447027d14ec2c541e82aae1014f08f812&tochange=2e1bfb7458de41a36432671c405eee62d897a761
6:01.47 INFO: Switching bisection method to taskcluster
6:01.47 INFO: Getting mozilla-central builds between c9c63f8447027d14ec2c541e82aae1014f08f812 and 2e1bfb7458de41a36432671c405eee62d897a761
6:02.84 WARNING: Skipping build 2e1bfb7458de: Unable to find build info using the taskcluster route 'gecko.v2.mozilla-central.shippable.revision.2e1bfb7458de41a36432671c405eee62d897a761.firefox.macosx64-opt'
...
6:08.91 INFO: There are no build artifacts for these changesets (they are probably too old).

This is a pretty easy fix; that really should never be null, but it looks like this code can cause that to happen:

https://searchfox.org/mozilla-central/rev/685203e4bc211073284f3c36f7f3d34f0953bb9c/dom/media/webrtc/sdp/SipccSdpAttributeList.cpp#794-795,798

We should probably not be calling PushEntry if we didn't recognize the type.

Set release status flags based on info from the regressing bug 1470568

Group: core-security → media-core-security
Keywords: sec-other
Assignee: nobody → docfaraday

This is interesting. This crash has nothing to do with rids, it has to do with an extra \r\n in the middle of the SDP.

Keeping this on our radar.

Yeah, this version, which doesn't insert spurious new lines, doesn't crash: https://jsfiddle.net/9x86guma/ .

I think this just boils down to having an fmtp line, but no rtpmap for that payload type. The extra \r\n was basically truncating the SDP, wiping out all the rtpmap lines. Boiling down a crashtest now.

Interesting. It does not happen if the rtpmap is simply absent. It has to exist, but after a stray \r\n.

Can we remove sec on this? This is just a content process nullptr crash.

Flags: needinfo?(continuation)
Group: media-core-security
Flags: needinfo?(continuation)
Keywords: sec-other
Pushed by bcampen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e7803147ce38
Test case for bug. r=ng
https://hg.mozilla.org/integration/autoland/rev/c6c731fb582d
If we cannot determine the type for an fmtp, do not add it to the list of successfully parsed fmtp attributes. r=ng
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 106 Branch
No longer regressed by: 1470568
No longer blocks: webrtc-triage
Regressed by: 1470568
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: