Crash in mozilla::MediaPipelineTransmit::ProcessVideoChunk

VERIFIED FIXED in Firefox 19

Status

()

Core
WebRTC
--
blocker
VERIFIED FIXED
6 years ago
4 years ago

People

(Reporter: standard8, Assigned: jesup)

Tracking

({crash, csectype-nullptr, regression})

unspecified
mozilla19
crash, csectype-nullptr, regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox18 unaffected, firefox19 verified, firefox-esr10 unaffected, firefox-esr17 unaffected)

Details

(Whiteboard: [adv-main19-], crash signature)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
This bug was filed from the Socorro interface and is 
report bp-2b8c9fea-f335-40d6-a553-837d42121018 .
============================================================= 

Turning on Video streaming crashes in today's (18th Oct) nightly build.

STR:

1) Visit http://people.mozilla.com/~anarayanan/webrtc/pc_test.html with the relevant prefs set to true.
2) Click Start
3) Share the Camera

=> Nightly Crashes.

Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dac5700acf8b&tochange=cb573b9307e5

Top of stack:

0 	XUL 	mozilla::MediaPipelineTransmit::ProcessVideoChunk 	ImageContainer.h:72
1 	XUL 	mozilla::MediaPipelineTransmit::PipelineListener::NotifyQueuedTrackChanges 	MediaPipeline.cpp:538
2 	XUL 	mozilla::MediaStreamGraphImpl::ExtractPendingInput 	MediaStreamGraph.cpp:612
3 	XUL 	mozilla::MediaStreamGraphImpl::RunThread 	MediaStreamGraph.cpp:1392
Created attachment 672781 [details]
callstack

Even though the testcase is public we should mark a security issue as such.
Group: core-security
(Assignee)

Comment 2

6 years ago
Known upstream issue with SRTP, already reported by Christoph.  There's a patch for upstream; I should apply the patch here; it didn't get included when we landed SRTP in m-c.

The read-past-end of this doesn't generate any actual leak of the data and was already noted in the source; so I don't feel this is a security bug.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 790083
(Reporter)

Comment 3

6 years ago
This still happens in today's nightly (2012-10-19):

https://crash-stats.mozilla.com/report/index/bp-034ec2e6-8cbb-4e38-a0fe-6d6a12121019

Built with: http://hg.mozilla.org/mozilla-central/rev/0ff60bfb3442 which includes the changeset landed on bug 790083.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Duplicate of this bug: 803527
(Assignee)

Comment 5

6 years ago
Comment on attachment 672781 [details]
callstack

This callstack is another bug.  The original report is still extant.

See also https://crash-stats.mozilla.com/report/index/bp-5e4ab5ed-7a7f-4586-ac2b-4c65e2121019
Attachment #672781 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Created attachment 673221 [details] [diff] [review]
Protect against NULL image chunks in NotifyQueuedTrackChanges()

This is really wallpaper over the bug until we figure out what and why it's happening, or if NULL images are actually allowed here
(Assignee)

Comment 7

6 years ago
Created attachment 673243 [details] [diff] [review]
Protect against NULL image chunks in NotifyQueuedTrackChanges()
(Assignee)

Updated

6 years ago
Attachment #673221 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Blocks: 801843
(Assignee)

Comment 8

6 years ago
Comment on attachment 673243 [details] [diff] [review]
Protect against NULL image chunks in NotifyQueuedTrackChanges()

When I modified getUserMedia to fix video insertion, I took roc's advice to send NULL images until the first one is available (the alternative was to insert a black frame, because otherwise MediaStreams will block until video is up, and audio will queue in the meantime.)  However, apparently there was always an unstated requirement that NotifyQueuedTrackChanges() can give you null images.  This should be documented.

Not easily testable without modifying NotifyPull() for video to force NULL frames.

Only an issue since I landed the patch for bug 801843 which added NULL frames.

We'll want to uplift this with bug 801843
Attachment #673243 - Flags: review?(ekr)

Comment 9

6 years ago
Comment on attachment 673243 [details] [diff] [review]
Protect against NULL image chunks in NotifyQueuedTrackChanges()

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

lgtm
Attachment #673243 - Flags: review?(ekr) → review+
(Assignee)

Comment 10

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c20ed5e12b8d
status-firefox18: --- → unaffected
status-firefox19: --- → affected
Target Milestone: --- → mozilla19
https://hg.mozilla.org/mozilla-central/rev/c20ed5e12b8d
Assignee: nobody → rjesup
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
status-firefox19: affected → fixed
Flags: in-testsuite?
Resolution: --- → FIXED

Updated

6 years ago
Keywords: verifyme
(In reply to Randell Jesup [:jesup] from comment #6)
> This is really wallpaper over the bug until we figure out what and why it's
> happening, or if NULL images are actually allowed here

Randell, shouldn't we keep this bug open then?
(Assignee)

Comment 13

6 years ago
That patch was obsoleted.  Fixed is correct.
Verified on 10/23 build by trying out the test case described in comment 0 a few times. Couldn't reproduce the crash.
Status: RESOLVED → VERIFIED
status-firefox19: fixed → verified

Updated

6 years ago
Keywords: verifyme

Updated

6 years ago
status-firefox-esr10: --- → unaffected

Updated

6 years ago
status-firefox-esr17: --- → unaffected
Whiteboard: [adv-main19-]
Keywords: csec-nullptr
Group: core-security
You need to log in before you can comment on or make changes to this bug.