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)

x86_64
All
defect
Not set
critical

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)

Attached file Repro-file (deleted) —
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 . . .
Severity: normal → critical
OS: Linux → All
Version: unspecified → Trunk
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?
(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.
Attachment #757023 - Attachment mime type: text/plain → text/html
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...
Attached patch Patch (v1) (deleted) — Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #757050 - Flags: review?(roc)
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Mass moving Web Audio bugs to the Web Audio component. Filter on duckityduck.
Component: Video/Audio → Web Audio
Flags: sec-bounty?
(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)
(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)
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-
Whiteboard: [adv-main24-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: