Closed Bug 791270 Opened 8 years ago Closed 8 years ago

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

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)

Attached file callstack
The parameter of addStream() is not properly validated.
Attached file testcase
Whiteboard: [WebRTC], [blocking-webrtc+]
Comment on attachment 669728 [details] [diff] [review]
Protect AddStream from NULL input


Change assert on null input to AddStream to be a runtime failure.

Test case should be changed to mozRTCPeerConnection to test on latest.
Attachment #669728 - Flags: review?(rjesup)
Assignee: nobody → ethanhugg
Comment on attachment 669728 [details] [diff] [review]
Protect AddStream from NULL input

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

Since xpconnect already throws an exception when we return an error here, r+
Attachment #669728 - Flags: review?(rjesup) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Has this been landed for Firefox 18? Please set the TM accordingly. Thanks.
Landed for 19, and we should consider for uplift to 18
Target Milestone: --- → mozilla19
Duplicate of this bug: 800130
Keywords: verifyme
Flags: in-testsuite?
Attached patch Crashtest v1Splinter Review
If this patch fixes the crash completely the following try run should be successful: https://tbpl.mozilla.org/?tree=Try&rev=da75e3bd42d9

Once the patch has been reviewed and we are green we are good to land on m-c.
Attachment #671657 - Flags: review?(rjesup)
(In reply to Henrik Skupin (:whimboo) from comment #10)
> Created attachment 671657 [details] [diff] [review]
> Crashtest v1
> 
> If this patch fixes the crash completely the following try run should be
> successful: https://tbpl.mozilla.org/?tree=Try&rev=da75e3bd42d9
> 
> Once the patch has been reviewed and we are green we are good to land on m-c.

If the try run passes, can mark this as verified?
Status: RESOLVED → VERIFIED
Keywords: verifyme
(In reply to Jason Smith [:jsmith] from comment #11)
> (In reply to Henrik Skupin (:whimboo) from comment #10)
> > Created attachment 671657 [details] [diff] [review]
> > Crashtest v1
> > 
> > If this patch fixes the crash completely the following try run should be
> > successful: https://tbpl.mozilla.org/?tree=Try&rev=da75e3bd42d9
> > 
> > Once the patch has been reviewed and we are green we are good to land on m-c.
> 
> If the try run passes, can mark this as verified?

Actually, change that. I just ran the attached test case and it looked alright.
Wait a second, didn't test this right.
Status: VERIFIED → RESOLVED
Closed: 8 years ago8 years ago
Keywords: verifyme
Now I did this right this time. Looks good.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Attachment #671657 - Flags: review?(rjesup) → review+
You need to log in before you can comment on or make changes to this bug.