New issue
Advanced search Search tips

Issue 390928 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2014
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security
Nag



Sign in to add a comment

Heap-use-after-free in v8::internal::GlobalHandles::Create

Reported by cloudfuz...@gmail.com, Jul 2 2014

Issue description

VULNERABILITY DETAILS
The attached testfile (crash.html) triggers a series of different crashes on the latest ASAN build of chrome (maybe a JIT issue?). The ogg file is required to trigger the event.

Multiple tabs make the test case more reliable and crashing it multiple times leads to different crash signature.

VERSION
Chrome Version: asan-symbolized-linux-release-280802
Operating System: Linux 64-bit

REPRODUCTION CASE
Attached

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: tab
Crash State: Different crashes as asan output in sym*.txt

 
crash.html
497 bytes View Download
sym3.txt
12.6 KB View Download
sym2.txt
9.9 KB View Download
sym.txt
16.0 KB View Download
mov_bbb.ogg
600 KB Download
Project Member

Comment 1 by ClusterFuzz, Jul 2 2014

ClusterFuzz is analyzing your testcase. See https://cluster-fuzz.appspot.com/testcase?key=5100809668788224
Project Member

Comment 2 by ClusterFuzz, Jul 3 2014

Labels: Stability-Memory-AddressSanitizer Security_Impact-Head
Status: Available
Project Member

Comment 3 by ClusterFuzz, Jul 3 2014

Summary: Heap-use-after-free in v8::internal::GlobalHandles::Create (was: Security: Different crashes with valueOf on Object prototype and audio event)
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5100809668788224

Uploader: felt@chromium.org
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x614000051f10
Crash State:
  - crash stack -
  v8::internal::GlobalHandles::Create
  void WebCore::DOMDataStore::setWrapper<WebCore::V8HTMLDocument, WebCore::HT
  - free stack -
  WebCore::GraphicsContext::~GraphicsContext
  WebCore::OpaqueRectTrackingContentLayerDelegate::paintContents
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=263040:263157

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95cK-Lns1L8RdSiiFRnEeqF9-tOyV8Reui2czthgOTqED6n5xwnlWAAUes1Y6JVQnYfhkDgEGd_lPD9YBFJEupw0iO24hT3BWlF0MX2-UmEG9XyakFw2PD_2IkY_Hgn8DHnNWuEulwBQm3fWE6PhcfwsHoTYZ1Va1cCoFCqWvPxCEz29N0


Cc: danno@chromium.org jkummerow@chromium.org
Owner: yangguo@chromium.org
Status: Assigned
Any idea what caused this from the regression range ?
Cc: yangguo@chromium.org
Owner: dcarney@chromium.org
Can reproduce with an x64 asan build.
Project Member

Comment 6 by ClusterFuzz, Jul 5 2014

Labels: Missing_Severity-1

Comment 7 by f...@chromium.org, Jul 6 2014

Labels: -Missing_Severity-1 Security_Severity-Medium
Project Member

Comment 8 by ClusterFuzz, Jul 6 2014

Labels: Pri-1
Labels: M-38
Bulk edit of medium severity issues impacting head to M-38.
Project Member

Comment 10 by ClusterFuzz, Jul 11 2014

Labels: ReleaseBlock-Beta Nag
This medium+ severity security issue is a regression on trunk.

Please fix this asap. If you are unable to look into this soon, please revert your change.

dcarney@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz

Comment 11 by laforge@google.com, Jul 14 2014

Labels: OS-All
Cc: jochen@chromium.org
Owner: jochen@chromium.org
on vacation - assigning to jochen
Labels: -Security_Severity-Medium -M-38 -ReleaseBlock-Beta Security_Severity-High M-36 ReleaseBlock-Stable
uh uh uh, that looks like a pretty bad use after free. We add persistent values to some hash map instead of replacing the old ones with new ones.

bumping labels

fix is simple: https://codereview.chromium.org/412843003
Cc: haraken@chromium.org
Labels: -Security_Impact-Head Security_Impact-Stable Security_Impact-Beta
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 23 2014

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

------------------------------------------------------------------
r178780 | jochen@chromium.org | 2014-07-23T19:58:47.776625Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/core/v8/V8PersistentValueMap.h?r1=178780&r2=178779&pathrev=178780

When overwriting a persistent handle, actually overwrite it

BUG= 390928 
R=haraken@chromium.org

Review URL: https://codereview.chromium.org/412843003
-----------------------------------------------------------------
Status: Fixed
Project Member

Comment 18 by bugdroid1@chromium.org, Jul 23 2014

Labels: Merge-TBD
Is there a merge required here?
Project Member

Comment 19 by ClusterFuzz, Jul 23 2014

Labels: -Restrict-View-SecurityTeam Merge-Triage M-37 Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Labels: -Merge-TBD Merge-Requested
requesting merge so we don't miss M36-p1
Project Member

Comment 21 by bugdroid1@chromium.org, Jul 24 2014

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

------------------------------------------------------------------
r178823 | jochen@chromium.org | 2014-07-24T08:07:31.891202Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/core/v8/SerializedScriptValue.cpp?r1=178823&r2=178822&pathrev=178823
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/core/v8/DOMWrapperMap.h?r1=178823&r2=178822&pathrev=178823

Replace further questionable HashMap::add usages in bindings

BUG= 390928 
R=dcarney@chromium.org

Review URL: https://codereview.chromium.org/411273002
-----------------------------------------------------------------
Which CLs are required here, r178780, r178823 or both?  If both, why do we have a second CL required the day after the first?
I think r178780 is enough to fix the issue. r178823 is a just-in-case fix.

No, we need both. DomWrapperMap had the same uaf
The reason for the second cl is that I searched for other sites exposing the same bug
Does this look good on canary?
Very good.
Labels: -Merge-Requested Merge-Approved
merge approved for m37 branch 2062
Project Member

Comment 29 by bugdroid1@chromium.org, Jul 30 2014

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

------------------------------------------------------------------
r179237 | jochen@chromium.org | 2014-07-30T15:28:35.330107Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/branches/chromium/2062/Source/bindings/v8/V8PersistentValueMap.h?r1=179237&r2=179236&pathrev=179237

Merge 178780 "When overwriting a persistent handle, actually ove..."

> When overwriting a persistent handle, actually overwrite it
> 
> BUG= 390928 
> R=haraken@chromium.org
> 
> Review URL: https://codereview.chromium.org/412843003

TBR=jochen@chromium.org

Review URL: https://codereview.chromium.org/426243003
-----------------------------------------------------------------
Project Member

Comment 30 by bugdroid1@chromium.org, Jul 30 2014

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

------------------------------------------------------------------
r179238 | jochen@chromium.org | 2014-07-30T15:31:52.394738Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/branches/chromium/2062/Source/bindings/v8/DOMWrapperMap.h?r1=179238&r2=179237&pathrev=179238
   M http://src.chromium.org/viewvc/blink/branches/chromium/2062/Source/bindings/v8/SerializedScriptValue.cpp?r1=179238&r2=179237&pathrev=179238

Merge 178823 "Replace further questionable HashMap::add usages i..."

> Replace further questionable HashMap::add usages in bindings
> 
> BUG= 390928 
> R=dcarney@chromium.org
> 
> Review URL: https://codereview.chromium.org/411273002

TBR=jochen@chromium.org

Review URL: https://codereview.chromium.org/429233002
-----------------------------------------------------------------
Labels: -Merge-Triage Release-0-M37
Labels: -Security_Impact-Beta reward-topanel
Labels: -reward-topanel reward-unpaid reward-4000
Thanks for the report! This qualifies for a $4000 reward. We decided to add a bonus to this one because it helped us identify a pattern for similar bugs.
Labels: -reward-unpaid reward-inprogress
Labels: -reward-inprogress reward-inprocess
Labels: -reward-inprocess
Processing via our e-payment system can take a few weeks, but reward should be on its way to you. Thanks again for your help!
Project Member

Comment 37 by ClusterFuzz, Oct 29 2014

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
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

Sign in to add a comment