The ConvolverNode only supports 1, 2 or 4 channels and must throw an exception for any other number of channels.
This causes a heap-buffer-overflow read in com.apple.WebKit.WebContent at com.apple.WebCore: WebCore::AudioArray<float>::copyToRange + 76 under ASan. Application Specific Information: ================================================================ ==30955==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200030a3f4 at pc 0x00010e94734a bp 0x7fff5360ed70 sp 0x7fff5360e528 READ of size 256 at 0x60200030a3f4 thread T0 #0 0x10e947349 in __asan_memcpy (/Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.11.xctoolchain/usr/lib/clang/7.0.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x39349) #1 0x11398935b in WebCore::AudioArray<float>::copyToRange(float const*, unsigned int, unsigned int) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x97f35b) #2 0x11503b0bf in WebCore::ReverbConvolverStage::ReverbConvolverStage(float const*, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, WebCore::ReverbAccumulationBuffer*, bool) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x20310bf) #3 0x11503b4b8 in WebCore::ReverbConvolverStage::ReverbConvolverStage(float const*, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, WebCore::ReverbAccumulationBuffer*, bool) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x20314b8) #4 0x11503921a in WebCore::ReverbConvolver::ReverbConvolver(WebCore::AudioChannel*, unsigned long, unsigned long, unsigned long, bool) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x202f21a) #5 0x115037bba in WebCore::Reverb::initialize(WebCore::AudioBus*, unsigned long, unsigned long, unsigned long, bool) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x202dbba) #6 0x1150376cc in WebCore::Reverb::Reverb(WebCore::AudioBus*, unsigned long, unsigned long, unsigned long, bool, bool) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x202d6cc) #7 0x115037d57 in WebCore::Reverb::Reverb(WebCore::AudioBus*, unsigned long, unsigned long, unsigned long, bool, bool) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x202dd57) #8 0x1133ca260 in WebCore::ConvolverNode::setBuffer(WebCore::AudioBuffer*, int&) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x3c0260) #9 0x114073996 in WebCore::setJSConvolverNodeBuffer(JSC::ExecState*, long long, long long) (/Users/local/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x1069996) #10 0x110359368 in JSC::callCustomSetter(JSC::ExecState*, bool (*)(JSC::ExecState*, long long, long long), bool, JSC::JSValue, JSC::JSValue) (/Users/local/OpenSource/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/JavaScriptCore+0x574368) [...]
<rdar://problem/27020410>
Created attachment 282306 [details] Patch v1
Please note that the patch for Bug 159232 needs to land before this one since it adds support to use anonymous functions and descriptive messages with shouldNotThrow() and shouldThrow().
Comment on attachment 282306 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=282306&action=review Looks good. You'll need to land this manually, since the commit-queue doesn't process security bugs. > Source/WebCore/ChangeLog:4 > + <https://webkit.org/b/159238> <rdar://problem/27020410> > LayoutTests/ChangeLog:4 > + <https://webkit.org/b/159238> <rdar://problem/27020410> > LayoutTests/webaudio/convolver-channels.html:18 > + for (var count = 1; count <= 32; ++count) { Just curious: Is 32 an arbitrary value? Or do we have Audio elements that use 32 channels?
Comment on attachment 282306 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=282306&action=review >> LayoutTests/webaudio/convolver-channels.html:18 >> + for (var count = 1; count <= 32; ++count) { > > Just curious: Is 32 an arbitrary value? Or do we have Audio elements that use 32 channels? I suspect it was chosen as a practical limit for the test.
Committed r202617: <http://trac.webkit.org/changeset/202617>
Comment on attachment 282306 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=282306&action=review > Source/WebCore/Modules/webaudio/ConvolverNode.cpp:137 > + bool isChannelCountGood = numberOfChannels == 1 || numberOfChannels == 2 || numberOfChannels == 4; Is it safe to drop the bufferLength check?
This will be rolled out as the fix wasn't complete.
Reverted r202617 for reason: The LayoutTest from this change crashes under GuardMalloc Committed r202645: <http://trac.webkit.org/changeset/202645>
Created attachment 282848 [details] Patch
Comment on attachment 282848 [details] Patch Re-landing after finding the corresponding Blink change to correct the buffer overrun found during our testing.
Committed r202845: <http://trac.webkit.org/changeset/202845>