999376 - Out-of-Bounds Read in WaveShaperNode::ProcessCurve

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[Holger Fuhrmannek]

Attachment: testcase.html

The testcase spawns the debugger with http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-04-22-03-02-04-mozilla-central/firefox-31.0a1.en-US.linux-x86_64-asan.tar.bz2.
The asan report is the original report from fuzzing, i'm not able to get the report again.

Stacktrace:

#0  0x00007fffe97b1030 in nsTArray_Impl<float, nsTArrayInfallibleAllocator>::ElementAt (this=<optimized out>, i=<optimized out>)
    at ../../dist/include/nsTArray.h:883
#1  0x00007fffeb45c233 in mozilla::dom::WaveShaperNodeEngine::ProcessCurve<1u> (this=<optimized out>, aInputBuffer=<optimized out>, 
    aOutputBuffer=<optimized out>) at /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/webaudio/WaveShaperNode.cpp:196
#2  0x00007fffeb45bd70 in mozilla::dom::WaveShaperNodeEngine::ProcessBlock (this=0x60b0000e5570, aStream=0x6150007b7080, aInput=..., 
    aOutput=<optimized out>, aFinished=0x7fffb1ef3700)
    at /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/webaudio/WaveShaperNode.cpp:227
#3  0x00007fffeb3435c3 in mozilla::AudioNodeStream::ProcessInput (this=0x6150007b7080, aFrom=<optimized out>, aFlags=<optimized out>, 
    aTo=<optimized out>) at /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/AudioNodeStream.cpp:430

...

Comment 1

[Holger Fuhrmannek]

Attachment: asan_report.txt

Comment 2

[Karl Tomlinson (:karlt)]

Out of bounds read at
https://hg.mozilla.org/mozilla-central/file/c962bde5ac0b/content/media/webaudio/WaveShaperNode.cpp#l209
Allocated at
https://hg.mozilla.org/mozilla-central/file/c962bde5ac0b/content/media/webaudio/WaveShaperNode.cpp#l315

Comment 3

[Karl Tomlinson (:karlt)]

The error is due to float not having enough precision to clamp accurately to the buffer length.  The length may be up to 2^31/4 = 2^27, while float has 24 bits of precision.  There may therefore be up to about 4 elements of error in the clamp, which would be 16 bytes after the end of the array.  The ASAN report says 24 bytes after the end of the array, so perhaps there's some situations where the rounding is not to nearest.

Due to the lack of precision, I don't see any way to read more than 4 bytes a short distance before and 4 bytes a short distance after a large allocation.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: use integers to clamp integer range in WaveShaper curve

float did not necessarily have enough precision.

Includes a -1 fix in the multiplier of the input signal to have
"a signal level of zero corresponding to the center value of the curve array".

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 8411588 [details] [diff] [review]
use integers to clamp integer range in WaveShaper curve

This isn't right when index is -ve and rounded up to 0.

Comment 6

[Karl Tomlinson (:karlt)]

(In reply to Karl Tomlinson (:karlt, back May 5) from comment #3)
> Due to the lack of precision, I don't see any way to read more than 4 bytes
> a short distance before and 4 bytes a short distance after a large
> allocation.

Actually there's no precision error with zero, so no way to read before the allocation, only after.

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: use integers to clamp maximum position in WaveShaper curve

Comment 8

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/32d79829ef99

Comment 9

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/mozilla-central/rev/32d79829ef99

Comment 10

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a8104912ebe

Comment 11

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/64f342488fbf

Comment 12

[Karl Tomlinson (:karlt)]

The large memory allocations seem to have resulted in processes being killed on B2G emulator and Android.

https://treeherder.mozilla.org/logviewer.html#?job_id=12231903&repo=mozilla-inbound
18:27:41 INFO - REFTEST TEST-LOAD | http://10.0.2.2:8888/tests/dom/media/test/crashtests/999376.html | 428 / 966 (44%)
18:27:41 INFO - Killed
18:27:41 WARNING - TEST-UNEXPECTED-FAIL | http://10.0.2.2:8888/tests/dom/media/test/crashtests/999376.html | application timed out after 330 seconds with no output

https://treeherder.mozilla.org/logviewer.html#?job_id=12232873&repo=mozilla-inbound
18:44:46 INFO - REFTEST TEST-LOAD | http://10.0.2.2:8854/tests/dom/media/test/crashtests/999376.html | 428 / 1437 (29%)
18:44:56 INFO - INFO | automation.py | Application ran for: 0:10:29.449203
18:44:56 INFO - INFO | zombiecheck | Reading PID log: /tmp/tmpTSalYApidlog
18:44:57 INFO - /data/tombstones does not exist; tombstone check skipped
18:44:58 INFO - WARNING | leakcheck | refcount logging is off, so leaks can't be detected!
18:44:58 INFO - REFTEST INFO | runreftest.py | Running tests: end.

Comment 13

[(no longer active)]

(In reply to Karl Tomlinson (ni?:karlt) from comment #12)
> The large memory allocations seem to have resulted in processes being killed
> on B2G emulator and Android.

Ugh.  I thought we were pretty careful to allocate all large audio buffers fallibly... :(