Closed Bug 791278 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: WebRTC: Signaling, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla19
Tracking Status
firefox18 --- affected

People

(Reporter: posidron, Assigned: ehugg)

References

Details

(Keywords: crash, testcase, Whiteboard: [WebRTC], [blocking-webrtc+])

Attachments

(4 files, 3 obsolete files)

Attached file testcase (obsolete) —
The parameter of SetLocalDescription() is not properly validated.
Attached file callstack (obsolete) —
Attached file testcase
Attachment #661251 - Attachment is obsolete: true
Attached file callstack
Attachment #661252 - Attachment is obsolete: true
Whiteboard: [WebRTC] → [WebRTC], [blocking-webrtc+]
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.
Attachment #669772 - Flags: review?(rjesup)
Attachment #669772 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/mozilla-central/rev/7e871133c607
Assignee: nobody → ethanhugg
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Keywords: verifyme
Status: RESOLVED → VERIFIED
Keywords: verifyme
Wait a sec, I didn't test this right.
Status: VERIFIED → RESOLVED
Closed: 8 years ago8 years ago
Keywords: verifyme
Verified. Definitely a candidate for a crash test.
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Keywords: verifyme
(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() {});
Attached patch Crashtest v1 (obsolete) — Splinter Review
Crashtest  which tests both setLocalDescription and setRemoteDescription for invalid callbacks.
Attachment #671971 - Flags: review?(rjesup)
Comment on attachment 671971 [details] [diff] [review]
Crashtest v1

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

No actual crashtest attached...
Attachment #671971 - Flags: review?(rjesup) → review-
Attached patch Crashtest v1Splinter Review
Sorry, I missed to hg qref the addition of the test.
Attachment #671971 - Attachment is obsolete: true
Attachment #672177 - Flags: review?(rjesup)
Attachment #672177 - Flags: review?(rjesup) → review+
Attachment #669772 - Attachment description: Protect setLocal/RemoteDesc from NULL input → Protect setLocal/RemoteDesc from NULL input [checked-in]
You need to log in before you can comment on or make changes to this bug.