[Webkit-unassigned] [Bug 139664] Crash with cyclic channel merger

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=139664

--- Comment #2 from mark.buer+webkitbugs at gmail.com ---
@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);

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141217/743dee9f/attachment-0002.html>