791278 - WebRTC crash [@sipcc::PeerConnectionImpl::SetLocalDescription]

Status: VERIFIED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: testcase

The parameter of SetLocalDescription() is not properly validated.

Comment 1

[Christoph Diehl [:posidron]]

Attachment: callstack

Comment 2

[Christoph Diehl [:posidron]]

Attachment: testcase

Comment 3

[Christoph Diehl [:posidron]]

Attachment: callstack

Comment 4

[Ethan Hugg [:ehugg]]

Attachment: Protect setLocal/RemoteDesc from NULL input [checked-in]

Comment 5

[Ethan Hugg [:ehugg]]

Comment on attachment 669772 [details] [diff] [review]
Protect setLocal/RemoteDesc from NULL input [checked-in]


I was unable to duplicate this one, but from the callstack (aSDP null) it appears it needs the same fix as 791270.  I changed both setLocal and setRemote.

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e871133c607

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7e871133c607

Comment 8

[Jason Smith [:jsmith]]

Wait a sec, I didn't test this right.

Comment 9

[Jason Smith [:jsmith]]

Verified. Definitely a candidate for a crash test.

Comment 10

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #9)
> Verified. Definitely a candidate for a crash test.

Henrik - Since you are already doing bug 791270, would you be interested in doing this one as well? If not, I'll wait for your patch there to land and then I'll get a patch out.

You basically need a crash test that does:

        var pc = new mozRTCPeerConnection();
        pc.setLocalDescription(function() {});

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Crashtest v1

Crashtest  which tests both setLocalDescription and setRemoteDescription for invalid callbacks.

Comment 12

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

Comment on attachment 671971 [details] [diff] [review]
Crashtest v1

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

No actual crashtest attached...

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Crashtest v1

Sorry, I missed to hg qref the addition of the test.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5a94cbed4b7a

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5a94cbed4b7a