New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Feb 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
link

Issue 172926: Heap-buffer-overflow in WebCore::AudioBufferSourceNode::process

Reported by attek...@gmail.com, Jan 29 2013

Issue description

Repro-file as attachment.

Tested on:

OS: Ubuntu 12.04
Chromium: ASAN 26.0.1398.0 (Developer Build 179318)

Test case is little unstable, but should crash if you wait some time.

ASAN-report:

==22680== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7ffed93c27c8 at pc 0x7ffeed080213 bp 0x7ffed7025190 sp 0x7ffed7025188
WRITE of size 8 at 0x7ffed93c27c8 thread T102 (AudioOutputDevic)
    #0 0x7ffeed080212 in WebCore::AudioBufferSourceNode::process(unsigned long) ???:0
    #1 0x7ffeec2132fc in WebCore::AudioNode::processIfNecessary(unsigned long) ???:0
    #2 0x7ffeec218fa9 in WebCore::AudioNodeOutput::pull(WebCore::AudioBus*, unsigned long) ???:0
    #3 0x7ffeed084077 in WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long) ???:0
    #4 0x7ffeef59d77d in WebCore::AudioPullFIFO::consume(WebCore::AudioBus*, unsigned long) ???:0
    #5 0x7ffeef39e456 in WebCore::AudioDestinationChromium::render(WebKit::WebVector<float*> const&, WebKit::WebVector<float*> const&, unsigned long) ???:0
.
.
.
allocated by thread T0 (chrome) here:
    #0 0x7ffee89f85c2 in operator new[](unsigned long) ??:0
    #1 0x7ffeed08157a in WebCore::AudioBufferSourceNode::setBuffer(WebCore::AudioBuffer*) ???:0
    #2 0x7ffef162882f in WebCore::V8AudioBufferSourceNode::bufferAccessorSetter(v8::Local<v8::String>, v8::Local<v8::Value>, v8::AccessorInfo const&) ???:0
    #3 0x7ffeefb0c69d in v8::internal::JSObject::SetPropertyWithCallback(v8::internal::Object*, v8::internal::String*, v8::internal::Object*, v8::internal::JSObject*, v8::internal::StrictModeFlag) ???:0
    #4 0x7ffeefb167a0 in v8::internal::JSObject::SetPropertyForResult(v8::internal::LookupResult*, v8::internal::String*, v8::internal::Object*, PropertyAttributes, v8::internal::StrictModeFlag, v8::internal::JSReceiver::StoreFromKeyed) ???:0
    #5 0x7ffeefb0b3e8 in v8::internal::JSReceiver::SetProperty(v8::internal::String*, v8::internal::Object*, PropertyAttributes, v8::internal::StrictModeFlag, v8::internal::JSReceiver::StoreFromKeyed) ???:0
.
.
.
 
chrome-heap-buffer-overflow-WebCoreAudioBufferSourceNodeprocess-213.html
2.4 KB View Download

Comment 1 by scarybea...@gmail.com, Jan 29 2013

Good job @attekett!
You seem to have hit a little nest of issues :-)

Comment 2 by attek...@gmail.com, Jan 29 2013

Thanks. :D I'll try to dig out few more. ;)

Comment 3 by cdn@chromium.org, Jan 31 2013

Cc: crogers@google.com james....@intel.com
Labels: -Pri-0 -Area-Undefined Pri-1 Area-WebKit Feature-Media-Audio SecSeverity-High SecImpacts-Stable SecImpacts-Beta OS-All
I suspect that the offending code is 

        for (unsigned i = 0; i < outputBus->numberOfChannels(); ++i)
            m_destinationChannels[i] = outputBus->channel(i)->mutableData();

It seems like if the AudioBus can have more channels than the AudioBuffer passed in to AudioBufferSourceNode::setBuffer().

I haven't been able to get this to repro yet on Windows but it seems fairly straight forward.

crogers and james.wei looks like this was added by you guys, please take a look.

Comment 4 by james....@intel.com, Jan 31 2013

cdn, thanks for reporting this issue. 

in AudioBufferSourceNode:: setBuffer(), if the channel of the buffer is not equal to that of AudioBus, the AudioBus should be reconfigured to have the same channel as the buffer. 

        output(0)->setNumberOfChannels(numberOfChannels);

void AudioNodeOutput::updateInternalBus()
{
    if (numberOfChannels() == m_internalBus->numberOfChannels())
        return;

    m_internalBus = adoptPtr(new AudioBus(numberOfChannels(), AudioNode::ProcessingSizeInFrames));

    // This may later be changed in pull() to point to an in-place bus with the same number of channels.
    m_actualDestinationBus = m_internalBus.get();
}


I will try to reproduce this issue and investigate it. thanks

Comment 5 by james....@intel.com, Jan 31 2013

I think I found the root cause of this issue. 

when setting the buffer to the AudioBufferSourceNode, the AudioContext will try to update the AudioOutputs and so re-configure AudioBus before rendering. 

But it is possible that AudioContext may fail to get the lock of the graph and renderring will start. 

So the channel number of m_buffer changed, but the AudioBus not re-configured and has different number of Channels.

Comment 6 by james....@intel.com, Jan 31 2013

it is by design that:
1. the audio thread should not be blocked with regular block. 
2. tryLock in audio thread may fail. 
3. AudioBus re-configuration happens in audio thread pre-rendering stage. 

should we change the design or just return if channel mis-match detected when rendering? 

Chris, what's your opinion? 
thanks

Comment 7 by cdn@chromium.org, Jan 31 2013

Owner: james....@intel.com
Status: Assigned
Thanks James. I did finally get this to reproduce on windows also although it took leaving it running overnight with a conditional breakpoint in the loop which hits when i >= the size of the m_destinationChannels[]. 

I also filed an upstream bug https://bugs.webkit.org/show_bug.cgi?id=108515

Comment 8 by james....@intel.com, Jan 31 2013

cdn, I can upload a patch to WebKit for review. but I am not authorized to access the bugzilla item. could you grant the access to me? my webkit accout is also james.wei@intel.com 

thanks

Comment 9 by infe...@chromium.org, Jan 31 2013

James, cced you.

Comment 10 by james....@intel.com, Feb 1 2013

inferno, thanks. I can access it now.

Comment 11 by james....@intel.com, Feb 1 2013

patch uploaded to webkit and cc croger and kbr for review.

Comment 12 by cdn@chromium.org, Feb 1 2013

Labels: Mstone-24

Comment 13 by parisa@chromium.org, Feb 11 2013

Hey James,

I'm following up on all the open high-severity security bugs since Pwnium/Pwn2Own (http://blog.chromium.org/2013/01/show-off-your-security-skills-pwn2own.html) is just around the corner (we're using M25).

How's this one going?

Comment 14 by crogers@google.com, Feb 12 2013

Status: Fixed
This should be resolved in WebKit:
https://bugs.webkit.org/show_bug.cgi?id=108515

Comment 15 by scarybea...@gmail.com, Feb 12 2013

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify reward-topanel Merge-Approved
Committed r141851: <http://trac.webkit.org/changeset/141851>

Thanks!

Comment 16 by scarybea...@gmail.com, Feb 20 2013

Labels: -Mstone-24 -Merge-Approved Mstone-25 Merge-Merged Release-1
M25: http://trac.webkit.org/changeset/143516

Comment 17 by scarybea...@gmail.com, Mar 1 2013

Labels: CVE-2013-0904

Comment 18 by scarybea...@gmail.com, Mar 2 2013

Labels: -reward-topanel reward-1000 reward-unpaid
$1000 !

Comment 19 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Type-Security -Area-WebKit -Feature-Media-Audio -SecSeverity-High -SecImpacts-Stable -SecImpacts-Beta -Mstone-25 Cr-Content Cr-Internals-Media-Audio Security-Impact-Stable Security-Impact-Beta Security-Severity-High M-25 Type-Bug-Security

Comment 20 by parisa@chromium.org, Mar 14 2013

Labels: -reward-unpaid reward-inprocess

Comment 21 by dalecur...@chromium.org, Mar 14 2013

I believe this is a similar problem to issue 188559.

Comment 22 by james....@intel.com, Mar 14 2013

@dalecurtis, could you grant the acces to issue #188559 to me? I can have a look at it. thansk

Comment 23 by dalecur...@chromium.org, Mar 14 2013

Done.

Comment 24 by james....@intel.com, Mar 14 2013

thanks

Comment 25 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Severity-High Security_Severity-High

Comment 26 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Impact-Stable Security_Impact-Stable

Comment 27 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Impact-Beta Security_Impact-Beta

Comment 28 by bugdroid1@chromium.org, Apr 5 2013

Project Member
Labels: -Cr-Content Cr-Blink

Comment 29 by parisa@chromium.org, Jun 24 2013

Labels: -reward-inprocess

Comment 30 by attek...@gmail.com, Jul 22 2013

Can this issue be opened to the public?

Comment 31 by jsc...@chromium.org, Nov 18 2013

Labels: -Restrict-View-SecurityNotify
Bulk release of old security bug reports.

Comment 32 by sheriffbot@chromium.org, Jun 14 2016

Project Member
Labels: -security_impact-beta

Comment 33 by sheriffbot@chromium.org, Oct 1 2016

Project Member
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 34 by sheriffbot@chromium.org, Oct 2 2016

Project Member
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 35 by mbarbe...@chromium.org, Oct 2 2016

Labels: allpublic

Comment 36 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-submitted

Sign in to add a comment