Closed Bug 1535868 Opened 6 months ago Closed 6 months ago

Negotiating DTLS without SRTP extension results in random crashes

Categories

(Core :: WebRTC: Networking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: drno, Assigned: drno)

Details

(Keywords: csectype-nullptr, sec-audit, Whiteboard: [adv-main68-])

Attachments

(1 file)

No description provided.

The problem appears to be that when we fail to negotiate the use_srtp extension the transportlayersrtp state gets set to error. But that error doesn't get propagated through the stack, as transportlayerdtls reports success. And transportlayersrtp only refuses to receive when being in an error state but keeps trying to send.

Proper handling in the sense that DTLS should fail if the upper layers need it, but can success if use_srtp is not needed because of a data channel only connection should be done in bug 1520692.

All of the crashes in comment 1 are nullptr crashes as far as I can tell.

I'm marking this as audit because they all look like null derefs, but adjust as needed.

Keywords: sec-audit
Group: core-security → media-core-security

I double checked the code in transportlayersrtp and I'm pretty sure this is the only corner case which wasn't covered yet. So I'm opening this bug now for the public.

Group: media-core-security
Pushed by aiakab@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/74d361ea2fe6
don't send SRTP when not negotiated. r=bwc
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Nils, do you think that this is a fix we should uplift to 67 beta?

Flags: needinfo?(drno)

(In reply to Pascal Chevrel:pascalc from comment #9)

Nils, do you think that this is a fix we should uplift to 67 beta?

Nils, ping :)

Sorry for the late reply.

As endpoints which try to negotiate something like this are pretty rare I think it's fine to let this ride the trains.

Flags: needinfo?(drno)
Whiteboard: [adv-main68-]
You need to log in before you can comment on or make changes to this bug.