Closed Bug 877527 Opened 8 years ago Closed 8 years ago

WebAudio heap-buffer-overflow crash [@mozilla::AudioBlockCopyChannelWithScale]

Categories

(Core :: Web Audio, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox22 --- disabled
firefox23 - disabled
firefox24 + fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: posidron, Assigned: padenot)

References

Details

(5 keywords, Whiteboard: [adv-main24-])

Attachments

(2 files)

Attached file testcase
content/media/AudioNodeEngine.cpp:64

void
AudioBlockCopyChannelWithScale(const float* aInput,
                               float aScale,
                               float* aOutput)
{
  if (aScale == 1.0f) {
*   memcpy(aOutput, aInput, WEBAUDIO_BLOCK_SIZE*sizeof(float));
[...]
}
Attached file callstack
Please see also: https://bugzilla.mozilla.org/show_bug.cgi?id=876207#c0

This is exactly where my concern is, we are not fixing the root of the problems.

I believe we should add a sanitize checks for the memcpy parameters so that we won't crash there again.
(In reply to Christoph Diehl [:cdiehl] from comment #2)
> Please see also: https://bugzilla.mozilla.org/show_bug.cgi?id=876207#c0
> 
> This is exactly where my concern is, we are not fixing the root of the
> problems.
> 
> I believe we should add a sanitize checks for the memcpy parameters so that
> we won't crash there again.

I don't think that's the case.  We can't check for the length of the buffer because AudioChunk doesn't keep track of that information at all.  We should just make sure we're consistent in keeping track of the number of channels, the offsets, etc.  There's no single root cause for these kinds of bugs, well, besides using an unsafe language!  :-)
Assignee: nobody → paul
This does not happen on tip for me, but I can repro on d7c6d6061ab5. Now bisecting to see what "fixed" it, because nothing seem to touch WebAudio in the log.
Paul, can you please check in the test case as a crashtest?

Closing as FIXED for now, for the purpose of tracking the work left.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(paul)
Resolution: --- → FIXED
Sure, will do later today.
Flags: needinfo?(paul)
Paul, please add this to the crashtest suite here:

<http://mxr.mozilla.org/mozilla-central/source/content/media/test/crashtests/>

See my recent commits to that directory for examples.  Thanks!
Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.
Component: Video/Audio → Web Audio
Whiteboard: [adv-main24-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.