966636 - Heap-buffer-overflow in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromInputBufferWithResampling

Status: VERIFIED FIXED

(Core :: Web Audio, defect, P1)

Description

[Atte Kettunen]

Attachment: Repro-file

Tested on: 

OS: Ubuntu  12.04

Firefox: ASAN build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1391177067/

ASAN-report:

==13712==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f426b940c7c at pc 0x7f42a49d0734 bp 0x7f4276c7fb30 sp 0x7f4276c7fb28
READ of size 4 at 0x7f426b940c7c thread T35 (MediaStreamGrph)
    #0 0x7f42a49d0733 in speex_resampler_process_float /builds/slave/m-cen-l64-asan-000000000000000/build/media/libspeex_resampler/src/resample.c:897:0
    #1 0x7f42a274c10f in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromInputBufferWithResampling(mozilla::AudioNodeStream*, mozilla::AudioChunk*, unsigned int, unsigned long, unsigned long, unsigned int, unsigned int&) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:215:0
    #2 0x7f42a274b640 in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromBuffer(mozilla::AudioNodeStream*, mozilla::AudioChunk*, unsigned int, unsigned int*, long*, unsigned int, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:333:0
    #3 0x7f42a274a243 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:424:0
    #4 0x7f42a26455e7 in mozilla::AudioNodeStream::ProduceOutput(long, long, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/AudioNodeStream.cpp:439:0
    #5 0x7f42a26bfc91 in ProduceDataForStreamsBlockByBlock /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/MediaStreamGraph.cpp:1098:0
.
.
.

Comment 1

[Karl Tomlinson (:karlt)]

Regression from https://hg.mozilla.org/mozilla-central/rev/9b1da46deff2

Read of up to 512 bytes with offset from allocation and allocation size controlled
by content.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: rename resampling copy variables consistently and test remaining input better

Comment 3

[Karl Tomlinson (:karlt)]

Comment on attachment 8369868 [details] [diff] [review]
rename resampling copy variables consistently and test remaining input better

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

This would require some effort to work out, but it is not too hard.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

The patch points to where the overflow happens, but not how to generate the overflow (switch buffers while processing latent samples from the resampler).

Which older supported branches are affected by this flaw?

28

If not all supported branches, which bug introduced the flaw?

bug 937057 

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Patches applies to 28 cleanly.

How likely is this patch to cause regressions; how much testing does it need?

Nothing complicated in the patch, so unlikely to regress.  There are several existing automated tests.

Comment 4

[Karl Tomlinson (:karlt)]

29 is also affected and patch applies.

Comment 5

[Al Billings [:abillings - ex-MoCo]]

Is 29 Aurora now with Trunk being 30?

Comment 6

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8369868 [details] [diff] [review]
rename resampling copy variables consistently and test remaining input better

sec-approval+ for trunk. Please nominate patch(es) for 29 and 28 after it goes in.

Comment 7

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba2c4b5ff7d8

Comment 8

[Carsten Book [:Tomcat]]

landed on central as https://hg.mozilla.org/mozilla-central/rev/ba2c4b5ff7d8

Comment 9

[Karl Tomlinson (:karlt)]

Comment on attachment 8369868 [details] [diff] [review]
rename resampling copy variables consistently and test remaining input better

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 937057 
User impact if declined: sec-high, comment 1
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): 
Nothing complicated in the patch, so unlikely to regress.  There are several existing automated tests.
String or IDL/UUID changes made by this patch: none

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/de63d702095e

Comment 11

[Ryan VanderMeulen [:RyanVM]]

This was landed on mozilla-beta by karlt and then automatically merged to b2g28. Apparently the bug wasn't marked after landing on mozilla-beta.

https://hg.mozilla.org/releases/mozilla-beta/rev/de63d702095e

Comment 12

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/280b795a39ac

Comment 14

[Matt Wobensmith [:mwobensmith][:matt:]]

Confirmed crash on ASan build of Fx29, 2013-12-19.
Verified fixed on ASan builds of Fx29, 30 and 31, 2014-04-17.

Comment 16

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/356dda972163

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/356dda972163