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

Issue 223962 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in WebCore::Reverb::latencyFrames

Project Member Reported by infe...@chromium.org, Mar 26 2013

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=174623476

Fuzzer: Attekett_webaudio_fuzzer

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x601600198e18
Crash State:
  - crash stack -
  WebCore::Reverb::latencyFrames
  WebCore::ConvolverNode::latencyTime
  - free stack -
  WebCore::ConvolverNode::setBuffer
  v8::internal::StoreCallbackProperty
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv963UOGM6m7-Ikip6VCdsBOhx1nkgvx4UVN73ewy8ukHON0gh-xQdAsQ_iiizQFppjfZhSico0zcttMSEKMVo9qIkNk_4qhXuAUn6ztyFmMNjhZPZNFosJaHk5BWYWA5P2YOL9c6GiJXrrnTMN46ixeAgR7NAmRkcbC6KmCc4U9oCNOK-ac

Additional requirements: Requires Interaction Gestures
 
Cc: attek...@gmail.com
Owner: james....@intel.com
Status: Assigned
Unreliable testcase, but free stack can give an idea on the bug.
Cc: crogers@google.com xingnan....@intel.com rtoy@chromium.org
See the thread numbers, freed in one thread and used in another.

READ of size 8 at 0x601600198e18 thread T7 (AudioOutputDevi)
    #0 0x7fc729aedfe9 in WebCore::Reverb::latencyFrames() const third_party/WebKit/Source/WTF/wtf/Vector.h:547
    #1 0x7fc72557971f in WebCore::ConvolverNode::latencyTime() const 
0x601600198e18 is located 8 bytes inside of 40-byte region [0x601600198e10,0x601600198e38)
freed by thread T0 (chrome) here:
    #0 0x7fc720c6ceb2 in operator delete(void*)
    #1 0x7fc72557926d in WebCore::ConvolverNode::setBuffer(WebCore::AudioBuffer*) third_party/WebKit/Source/WTF/wtf/OwnPtr.h:141
    #2 0x7fc724160c25 in v8::internal::StoreCallbackProperty(v8::internal::Arguments, v8::internal::Isolate*) v8/src/stub-cache.cc:1048

Comment 4 by kcc@chromium.org, Mar 27 2013

Cc: dvyukov@chromium.org glider@chromium.org
can we try this with tsan v2? 

Comment 5 by glider@chromium.org, Mar 27 2013

There's a whole bunch of TSan v2 races, some include WebCore::Reverb and other media-related code. Will take a look.
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=175159532

Fuzzer: Attekett_webaudio_fuzzer

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x600a000c0af8
Crash State:
  - crash stack -
  WebCore::Reverb::latencyFrames
  WebCore::ConvolverNode::latencyTime
  - free stack -
  WebCore::ConvolverNode::setBuffer
  v8::internal::JSObject::SetPropertyWithCallback
  


Additional requirements: Requires Interaction Gestures
Project Member

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

Labels: -Cr-Content Cr-Blink

Comment 8 by cdn@chromium.org, Apr 16 2013

Labels: Cr-Blink-Audio

Comment 9 by cdn@chromium.org, Apr 16 2013

James, do you plan to contribute to Blink? If not we will need to find new owners for these security bugs.

Thanks!
 will investigate this issue after this patch landed: https://codereview.chromium.org/14042005/
Status: WontFix
Don't see this anymore. We won't have a way to verify this one-time-crasher as duplicate, unless it hits again. Closing.
Labels: -reward-topanel
Labels: External-Fuzzer-Contribution
Project Member

Comment 14 by ClusterFuzz, Sep 3 2013

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6724805919244288

Fuzzer: Attekett_webaudio_fuzzer
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x611000c9e394
Crash State:
  - crash stack -
  WebCore::Reverb::latencyFrames
  WebCore::ConvolverNode::latencyTime
  - free stack -
  WebCore::ConvolverNode::setBuffer
  WebCore::ConvolverNodeV8Internal::bufferAttributeSetterCallback
  


Unreliable crash found using linux_tsan_chrome_mp job type (history_size=6).
Additional requirements: Requires Interaction Gestures


Owner: rtoy@chromium.org
Status: Assigned
Cc: gregsimon@chromium.org
Labels: M-29 Security_Impact-Beta Security_Impact-Stable
Labels: -Security_Severity-High Security_Severity-Medium
Fixing severity based on the fact, that all of these are race conditions (free, crash on different threads). No reliable reproducer.
Cc: haraken@chromium.org
The culprit would be that we don't protect the access to m_reverb with MutexLock.

double ConvolverNode::latencyTime() const
{
    // MutexLocker is missing. Thus thread conflicts.
    return m_reverb ? m_reverb->latencyFrames() / static_cast<double>(sampleRate()) : 0;
}

I'll write a CL soon.
Owner: haraken@chromium.org
Cc: kbr@chromium.org
Project Member

Comment 22 by bugdroid1@chromium.org, Sep 5 2013

The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=157245

------------------------------------------------------------------------
r157245 | haraken@chromium.org | 2013-09-04T21:23:14.917120Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/webaudio/ConvolverNode.cpp?r1=157245&r2=157244&pathrev=157245

Fix threading races on ConvolverNode::m_reverb in ConvolverNode::latencyFrames()

According to the crash report (https://cluster-fuzz.appspot.com/testcase?key=6515787040817152),
ConvolverNode::m_reverb races between ConvolverNode::latencyFrames() and ConvolverNode::setBuffer().
This CL adds a proper lock for ConvolverNode::m_reverb.

BUG= 223962 
No tests because the crash depends on threading races and thus not reproducible.

Review URL: https://chromiumcodereview.appspot.com/23514037
------------------------------------------------------------------------
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved reward-topanel
Status: Fixed
I will keep monitoring the CF for this webaudio crash once this blink change rolls into chromium. 
Please merge your change to the m30 branch (1599) by early next week [using drover]. We have m30 beta coming next week and we want all the security changes in by that time. 
Project Member

Comment 25 by bugdroid1@chromium.org, Sep 12 2013

Labels: -Merge-Approved merge-merged-1599
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=157690

------------------------------------------------------------------------
r157690 | haraken@chromium.org | 2013-09-12T19:21:20.634216Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/branches/chromium/1599/Source/modules/webaudio/ConvolverNode.cpp?r1=157690&r2=157689&pathrev=157690

Merge 157245 "Fix threading races on ConvolverNode::m_reverb in ..."

> Fix threading races on ConvolverNode::m_reverb in ConvolverNode::latencyFrames()
> 
> According to the crash report (https://cluster-fuzz.appspot.com/testcase?key=6515787040817152),
> ConvolverNode::m_reverb races between ConvolverNode::latencyFrames() and ConvolverNode::setBuffer().
> This CL adds a proper lock for ConvolverNode::m_reverb.
> 
> BUG= 223962 
> No tests because the crash depends on threading races and thus not reproducible.
> 
> Review URL: https://chromiumcodereview.appspot.com/23514037

TBR=haraken@chromium.org

Review URL: https://codereview.chromium.org/24123002
------------------------------------------------------------------------
Merged into M30.
Labels: Release-0
Labels: -M-29 M-30 Merge-Merged
Did you saw our new criteria for possibly issuing higher rewards? See http://www.chromium.org/Home/chromium-security/vulnerability-rewards-program/reward-nomination-process
E.g. If you are able to provide a repro that faulted at an address of 0x41414141, it will qualify for the new higher rewards. Or, if you can show that you have control between free and crash points, etc.
Labels: CVE-2013-2906
Labels: -reward-topanel reward-500 reward-unpaid
@attekett: thanks, we'll be tagging most of these racy bugs at $500. If ever you were able to get the repro deterministic and demonstrate a controllable corruption, there's the liklihood that the new reward rules would enable a higher reward.
Labels: -reward-unpaid reward-inprocess
Project Member

Comment 33 by ClusterFuzz, Feb 6 2014

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Labels: -reward-inprocess
Labels: Stability-ThreadSanitizer
Project Member

Comment 36 by ClusterFuzz, Feb 2 2016

Labels: -Security_Impact-Beta
Components: -Blink>Audio Blink>Media>Audio
Renaming Blink>Audio to Blink>Media>Audio for better characterization
Project Member

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

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
Project Member

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

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
Labels: allpublic
Labels: CVE_description-submitted

Sign in to add a comment