Crash with cyclic channel merger

• Component: Web Audio
• Status: NEW ---

This snippet of JS code run from the developer console of Safari:

var audioContext;
if (!audioContext) {
    if (window.AudioContext) {
        audioContext = new AudioContext();
    } else if (window.webkitAudioContext) {
        audioContext = new webkitAudioContext();  
    } else {
        throw new Error('AudioContext missing');
    }
}

var o = audioContext.createOscillator();
var m = audioContext.createChannelMerger(2);
var d = audioContext.createDelay();

o.connect(m, 0, 1);
d.connect(m, 0, 0);
m.connect(d);
m.connect(audioContext.destination);
d.delayTime.value = 0.5;
o.start(0);


causes a crash with this relevant stack trace:
Thread 22 Crashed:: com.apple.audio.IOThread.client
0   com.apple.WebCore               0x000000010f5b99f6 WebCore::AudioNodeOutput::updateInternalBus() + 22
1   com.apple.WebCore               0x000000010f5b991a WebCore::AudioNodeOutput::updateNumberOfChannels() + 42
2   com.apple.WebCore               0x000000010f5e59b9 WebCore::ChannelMergerNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 89
3   com.apple.WebCore               0x000000010f5b9990 WebCore::AudioNodeOutput::updateNumberOfChannels() + 160
4   com.apple.WebCore               0x000000010f5aea82 WebCore::AudioBasicProcessorNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 114
5   com.apple.WebCore               0x000000010f5b9990 WebCore::AudioNodeOutput::updateNumberOfChannels() + 160
6   com.apple.WebCore               0x000000010f5e59b9 WebCore::ChannelMergerNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 89
7   com.apple.WebCore               0x000000010f5b9990 WebCore::AudioNodeOutput::updateNumberOfChannels() + 160
8   com.apple.WebCore               0x000000010f5aea82 WebCore::AudioBasicProcessorNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 114
etc, etc
128 com.apple.WebCore               0x000000010f5aea82 WebCore::AudioBasicProcessorNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 114
129 com.apple.WebCore               0x000000010f5b9990 WebCore::AudioNodeOutput::updateNumberOfChannels() + 160
130 com.apple.WebCore               0x000000010f5e59b9 WebCore::ChannelMergerNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 89
131 com.apple.WebCore               0x000000010f5bd330 WebCore::AudioSummingJunction::updateRenderingState() + 384
132 com.apple.WebCore               0x000000010f5b5078 WebCore::AudioContext::handleDirtyAudioSummingJunctions() + 120
133 com.apple.WebCore               0x000000010f5b4fd1 WebCore::AudioContext::handlePreRenderTasks() + 129
134 com.apple.WebCore               0x000000010f5b7488 WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long) + 136
135 com.apple.WebCore               0x000000010f5b6f72 WebCore::AudioDestinationMac::render(unsigned int, AudioBufferList*) + 82
136 com.apple.WebCore               0x000000010f5b6eaf WebCore::AudioDestinationMac::inputProc(void*, unsigned int*, AudioTimeStamp const*, unsigned int, unsigned int, AudioBufferList*) + 15
137 com.apple.audio.units.Components    0x000000012182c039 AUInputElement::PullInput(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int) + 177
138 com.apple.audio.units.Components    0x000000012182b8ef AUInputFormatConverter2::InputProc(OpaqueAudioConverter*, unsigned int*, AudioBufferList*, AudioStreamPacketDescription**, void*) + 193
139 com.apple.audio.toolbox.AudioToolbox    0x00007fff8ea0c185 AudioConverterChain::CallInputProc(unsigned int) + 417
140 com.apple.audio.toolbox.AudioToolbox    0x00007fff8ea0bed1 AudioConverterChain::FillBufferFromInputProc(unsigned int*, CABufferList*) + 125
141 com.apple.audio.toolbox.AudioToolbox    0x00007fff8e9eaf19 BufferedAudioConverter::GetInputBytes(unsigned int, unsigned int&, CABufferList const*&) + 179
142 com.apple.audio.toolbox.AudioToolbox    0x00007fff8e9c9c9a CBRConverter::RenderOutput(CABufferList*, unsigned int, unsigned int&, AudioStreamPacketDescription*) + 104
143 com.apple.audio.toolbox.AudioToolbox    0x00007fff8e9eada0 BufferedAudioConverter::FillBuffer(unsigned int&, AudioBufferList&, AudioStreamPacketDescription*) + 286
144 com.apple.audio.toolbox.AudioToolbox    0x00007fff8ea0bce9 AudioConverterChain::RenderOutput(CABufferList*, unsigned int, unsigned int&, AudioStreamPacketDescription*) + 99
145 com.apple.audio.toolbox.AudioToolbox    0x00007fff8e9eada0 BufferedAudioConverter::FillBuffer(unsigned int&, AudioBufferList&, AudioStreamPacketDescription*) + 286
146 com.apple.audio.toolbox.AudioToolbox    0x00007fff8e9c94ee AudioConverterFillComplexBuffer + 292
147 com.apple.audio.units.Components    0x000000012182b7a6 AUInputFormatConverter2::PullAndConvertInput(AudioTimeStamp const&, unsigned int&, AudioBufferList&, AudioStreamPacketDescription*, bool&) + 98
148 com.apple.audio.units.Components    0x000000012182b08a AUConverterBase::RenderBus(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int) + 188
149 com.apple.audio.units.Components    0x0000000121828c61 AUBase::DoRenderBus(unsigned int&, AudioTimeStamp const&, unsigned int, AUOutputElement*, unsigned int, AudioBufferList&) + 153
150 com.apple.audio.units.Components    0x0000000121827515 AUBase::DoRender(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int, AudioBufferList&) + 423
151 com.apple.audio.units.Components    0x000000012182e878 AUHAL::AUIOProc(unsigned int, AudioTimeStamp const*, AudioBufferList const*, AudioTimeStamp const*, AudioBufferList*, AudioTimeStamp const*, void*) + 2076
152 com.apple.audio.CoreAudio       0x00007fff8bdc49db HALC_ProxyIOContext::IOWorkLoop() + 3667
153 com.apple.audio.CoreAudio       0x00007fff8bdc3add HALC_ProxyIOContext::IOThreadEntry(void*) + 97
154 com.apple.audio.CoreAudio       0x00007fff8bdc399d HALB_IOThread::Entry(void*) + 75
155 libsystem_pthread.dylib         0x00007fff8fe2b899 _pthread_body + 138
156 libsystem_pthread.dylib         0x00007fff8fe2b72a _pthread_start + 137
157 libsystem_pthread.dylib         0x00007fff8fe2ffc9 thread_start + 13


Guessing at the cause, the line at https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp#L74 probably results in a null audio bus being returned at some point (after several renders?) due to an excessive number of channels.

If this guess is correct, an easy fix might be to modify https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp#L74 to always create an audio bus with capped number of channels.

ie:
m_internalBus = AudioBus::create(std::min(numberOfChannels(), AudioContext::maxNumberOfChannels()), AudioNode::ProcessingSizeInFrames);

[Alexey Proskuryakov]

This looks like a runaway recursion to me.

@Alexey
That was my first thought too. But, there aren't very many stack frames in total and the final frame is different from previous frames, meaning it has reached an endpoint for the recursion.


This doesn't look to be a security concern as it is a null pointer dereference (not a dangling pointer dereference). The null pointer is set when an attempt is made to create an AudioBus with too many channels as seen here https://github.com/WebKit/webkit/blob/master/Source/WebCore/platform/audio/AudioBus.cpp#L53 . Some time later, an attempt is made to dereference that null pointer, which causes the crash.


A fix appears to have been applied to Blink, it's a shame it wasn't thrown over the wall to WebKit at the same time. I cannot find the changeset for this fix.

A check against the channel count is performed here:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp&q=channelmerger&sq=package:chromium&l=118
that is missing from here:
https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/webaudio/ChannelMergerNode.cpp#L105
Applying this one-line change to my local checkout of WebKit confirms that it addresses the bug.


I've raised a bug against the specification for this scenario of ChannelMergerNodes in a cycle: https://github.com/WebAudio/web-audio-api/issues/459


I've attached a debugging session showing that this bug is indeed a null pointer dereference (and not a dangling pointer dereference), thus substantiating my "probably not a security bug" claim from earlier:

(lldb) thread step-over
Process 70149 stopped
* thread #17: tid = 0x1dbbbb, 0x000000010c3c3bc8 WebCore`WebCore::AudioNodeOutput::updateInternalBus(this=0x00007fd97b218f80) + 24 at AudioNodeOutput.cpp:71, name = 'com.apple.audio.IOThread.client', stop reason = step over
    frame #0: 0x000000010c3c3bc8 WebCore`WebCore::AudioNodeOutput::updateInternalBus(this=0x00007fd97b218f80) + 24 at AudioNodeOutput.cpp:71
   68  	
   69  	void AudioNodeOutput::updateInternalBus()
   70  	{
-> 71  	    if (numberOfChannels() == m_internalBus->numberOfChannels())
   72  	        return;
   73  	
   74  	    m_internalBus = AudioBus::create(numberOfChannels(), AudioNode::ProcessingSizeInFrames);
(lldb) frame variable this->m_numberOfChannels
(unsigned int) this->m_numberOfChannels = 34
(lldb) frame variable this->m_internalBus
(WTF::RefPtr<WebCore::AudioBus>) this->m_internalBus = {
  m_ptr = 0x0000000000000000
}
(lldb) thread step-over
Process 70149 stopped
* thread #17: tid = 0x1dbbbb, 0x000000010c3c3bc8 WebCore`WebCore::AudioNodeOutput::updateInternalBus(this=0x00007fd97b218f80) + 24 at AudioNodeOutput.cpp:71, name = 'com.apple.audio.IOThread.client', stop reason = EXC_BAD_ACCESS (code=1, address=0x1c)
    frame #0: 0x000000010c3c3bc8 WebCore`WebCore::AudioNodeOutput::updateInternalBus(this=0x00007fd97b218f80) + 24 at AudioNodeOutput.cpp:71
   68  	
   69  	void AudioNodeOutput::updateInternalBus()
   70  	{
-> 71  	    if (numberOfChannels() == m_internalBus->numberOfChannels())
   72  	        return;
   73  	
   74  	    m_internalBus = AudioBus::create(numberOfChannels(), AudioNode::ProcessingSizeInFrames);

[Radar WebKit Bug Importer]

<rdar://problem/19280969>

[Ahmad Saleem]

Potentially following commit?
https://src.chromium.org/viewvc/blink?view=revision&revision=171678