Closed
Bug 878478
Opened 11 years ago
Closed 11 years ago
Heap-buffer-overflow READ in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromInputBuffer
Categories
(Core :: Web Audio, defect)
Tracking
()
RESOLVED
FIXED
mozilla24
Tracking | Status | |
---|---|---|
firefox21 | --- | unaffected |
firefox22 | --- | disabled |
firefox23 | --- | disabled |
firefox24 | --- | fixed |
firefox-esr17 | --- | unaffected |
b2g18 | --- | unaffected |
People
(Reporter: attekett, Assigned: ehsan.akhgari)
References
Details
(4 keywords, Whiteboard: [adv-main24-])
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
Tested on:
OS: Ubuntu 12.04
Firefox: ASAN opt-build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1370082293/
ASAN-report:
==13806== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fb8aa796e50 at pc 0x7fb8ce920d22 bp 0x7fb8a2494070 sp 0x7fb8a2494068
READ of size 1 at 0x7fb8aa796e50 thread T28
#0 0x7fb8ce920d21 in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromInputBuffer(mozilla::AudioChunk*, unsigned int, unsigned long, unsigned long, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:174
#1 0x7fb8ce91f16b in mozilla::dom::AudioBufferSourceNodeEngine::ProduceAudioBlock(mozilla::AudioNodeStream*, mozilla::AudioChunk const&, mozilla::AudioChunk*, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:397
#2 0x7fb8ce88704a in mozilla::AudioNodeStream::ProduceOutput(long, long) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/AudioNodeStream.cpp:428
#3 0x7fb8ce8f5a1e in mozilla::AudioNodeStream::SampleRate() const /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/MediaStreamGraph.cpp:967
#4 0x7fb8ce9089a5 in mozilla::(anonymous namespace)::MediaStreamGraphThreadRunnable::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/MediaStreamGraph.cpp:1214
#5 0x7fb8d0fc9cd2 in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/xpcom/build/nsThreadUtils.cpp:238
.
.
.
Updated•11 years ago
|
Severity: normal → critical
OS: Linux → All
Version: unspecified → Trunk
Comment 1•11 years ago
|
||
Yet again we are crashing at the same memcpy() with a different testcase as we did already in bug 876338. Is there really no possible way to sanitize those arguments before we make this particular memcpy() call?
Updated•11 years ago
|
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to Christoph Diehl [:cdiehl] from comment #1)
> Yet again we are crashing at the same memcpy() with a different testcase as
> we did already in bug 876338. Is there really no possible way to sanitize
> those arguments before we make this particular memcpy() call?
No, there is no silver bullet. Sorry.
Assignee | ||
Updated•11 years ago
|
Attachment #757023 -
Attachment mime type: text/plain → text/html
Assignee | ||
Comment 3•11 years ago
|
||
So here's what happens here. We first set a buffer, then we set it to looping. Now, since loopEnd is not set, we set it to the length of the first buffer. Then we set loopEnd, but this time loopEnd is so small that loopStartTicks and loopEndTicks will both end up being 0, hence in this condition check <http://mxr.mozilla.org/mozilla-central/source/content/media/webaudio/AudioBufferSourceNode.cpp?force=1#677> we decide to not change the loop arguments, and then we go ahead and set a buffer with a smaller size, which means that loopEnd will now be too large of an offset...
Assignee | ||
Comment 4•11 years ago
|
||
Attachment #757050 -
Flags: review?(roc) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status-firefox24:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Assignee | ||
Comment 7•11 years ago
|
||
Mass moving Web Audio bugs to the Web Audio component. Filter on duckityduck.
Component: Video/Audio → Web Audio
Updated•11 years ago
|
status-firefox21:
--- → unaffected
status-firefox22:
--- → disabled
status-firefox23:
--- → affected
status-firefox-esr17:
--- → unaffected
Updated•11 years ago
|
Flags: sec-bounty?
Comment 8•11 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #3)
> We first set a buffer, then we set it to looping. [...]
> and then we go ahead and set a buffer with a smaller size, which
> means that loopEnd will now be too large of an offset...
What are we doing in the loop? Reading or writing? If we're reading, what do we do with the potentially bogus values we just read?
Flags: needinfo?(ehsan)
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Daniel Veditz [:dveditz] from comment #8)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #3)
> > We first set a buffer, then we set it to looping. [...]
> > and then we go ahead and set a buffer with a smaller size, which
> > means that loopEnd will now be too large of an offset...
>
> What are we doing in the loop? Reading or writing?
We're reading from it.
> If we're reading, what do
> we do with the potentially bogus values we just read?
To the best of my knowledge, we just do floating point computations on it, not making branching decisions. Which means that we can crash if we get a math exception, but not jump to the wrong address.
Flags: needinfo?(ehsan)
Comment 10•11 years ago
|
||
So if you manage not to crash the browser (heap feng shui) you might be able to interpret memory contents by analyzing the end of a generated sound? Could be interesting, but a much less severe problem than arbitrary code execution.
Flags: sec-bounty? → sec-bounty-
Keywords: sec-critical → sec-moderate
Updated•11 years ago
|
Updated•11 years ago
|
Whiteboard: [adv-main24-]
Updated•11 years ago
|
status-b2g18:
--- → unaffected
Updated•11 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•