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 0 users

Issue metadata

Status: Fixed
Owner:
Leaves the project on 2018/03/02
Closed: Oct 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in blink::XMLHttpRequest::handleRequestError

Project Member Reported by ClusterFuzz, Oct 8 2014

Issue description

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

Fuzzer: Kkania_chromebot
Job Type: Linux_asan_chrome_media

Crash Type: Heap-use-after-free READ 1
Crash Address: 0x6190001661b8
Crash State:
  blink::XMLHttpRequest::handleRequestError
  blink::XMLHttpRequest::didFail
  blink::DocumentThreadableLoader::handleResponse
  

Minimized Testcase (0.11 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96KwIMKHBQmYpJSD2KNHLElOSoeEZBqjNVp13BNYDRyqi6pOcG6CsTh50MdOear8z_dBwMftrpi9cC2xFHejwjHr4zT_I09XOLI-l1z6OjISN-2cKYeSegxdvKkPTq0vzDwgfi0WJZXmATHY_5w6Gx9gie7Zw

Filer: inferno
 
Labels: Security_Impact-Stable
Owner: tyoshino@chromium.org
Status: Assigned
Project Member

Comment 2 by ClusterFuzz, Oct 8 2014

Labels: M-38 Pri-1
Project Member

Comment 3 by ClusterFuzz, Oct 8 2014

ClusterFuzz is analyzing your testcase. Chromium developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=5733166592557056
Just a fyi, it took like 51 seconds for crash to happen [Clusterfuzz-linux-2236: Crash detected in r99 [51 seconds]]
Project Member

Comment 5 by ClusterFuzz, Oct 13 2014

ClusterFuzz has detected this issue as fixed in latest custom build.

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

Fuzzer: Kkania_chromebot
Job Type: Linux_asan_chrome_media

Crash Type: Heap-use-after-free READ 1
Crash Address: 0x6190001661b8
Crash State:
  blink::XMLHttpRequest::handleRequestError
  blink::XMLHttpRequest::didFail
  blink::DocumentThreadableLoader::handleResponse
  

Minimized Testcase (0.11 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96KwIMKHBQmYpJSD2KNHLElOSoeEZBqjNVp13BNYDRyqi6pOcG6CsTh50MdOear8z_dBwMftrpi9cC2xFHejwjHr4zT_I09XOLI-l1z6OjISN-2cKYeSegxdvKkPTq0vzDwgfi0WJZXmATHY_5w6Gx9gie7Zw

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.

Project Member

Comment 6 by ClusterFuzz, Oct 16 2014

Labels: Nag
tyoshino@: 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
Sorry. I was on biztrip and sick after that. I'll take a look.
It's UaF on m_async. That is UaF on the XMLHttpRequest instance. I guess the XHR object is destroyed in internalAbort(). Its reference count becomes 1 via loader->cancel() call, and when we're back and exit internalAbort() method, |protect| is released and XHR's refcount becomes 0 and gets destroyed. Then, we synchronously call handleRequestError() after internalAbort(), maybe in either of handleDidCancel(), handleNetworkError() or handleDidTimeout().

Labels: WIP
Status: Started
https://codereview.chromium.org/663693003/
Cc: kouhei@chromium.org
Feel free to reassign to me when you are sick/busy.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 17 2014

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

------------------------------------------------------------------
r183859 | tyoshino@chromium.org | 2014-10-17T05:43:22.969853Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/xml/XMLHttpRequest.cpp?r1=183859&r2=183858&pathrev=183859

Place RefPtr for protection not in internalAbort() but in its caller.

We have RefPtr for preventing XHR from being destroyed in
internalAbort(). But we're touching XHR's member variables in the caller
of internalAbort(). We must have the RefPtr for protection there.

BUG= 421504 

Review URL: https://codereview.chromium.org/663693003
-----------------------------------------------------------------
inferno: For security fix, we can set merge-request immediately?
Status: Fixed
Nope, we need to wait for some bake time. Look for sheriffbot comment after we mark it Fixed.
Project Member

Comment 15 by ClusterFuzz, Oct 17 2014

Labels: -Restrict-View-SecurityTeam Merge-Triage M-39 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
Project Member

Comment 16 by ClusterFuzz, Oct 17 2014

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-Requested
Dev released on 2014-10-21 17:46:02.678500 from Branch 2194 at Blink 183946 (base) includes the CL. No new crash observed.
Labels: -Merge-Requested Merge-Approved
merge approved for m39 branch 2171
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 23 2014

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

------------------------------------------------------------------
r184241 | tyoshino@chromium.org | 2014-10-23T08:19:26.631978Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/branches/chromium/2171/Source/core/xml/XMLHttpRequest.cpp?r1=184241&r2=184240&pathrev=184241

Merge 183859 "Place RefPtr for protection not in internalAbort()..."

> Place RefPtr for protection not in internalAbort() but in its caller.
> 
> We have RefPtr for preventing XHR from being destroyed in
> internalAbort(). But we're touching XHR's member variables in the caller
> of internalAbort(). We must have the RefPtr for protection there.
> 
> BUG= 421504 
> 
> Review URL: https://codereview.chromium.org/663693003

TBR=tyoshino@chromium.org

Review URL: https://codereview.chromium.org/667213003
-----------------------------------------------------------------
Labels: -M-39 Merge-Requested
Beta build successful with no new error.
Labels: -M-38 -Nag -WIP -Merge-Triage -Merge-Requested Release-0-M39
Labels: Cr-Blink-XHR
Looks resolved

Query:
REGEXP(product.name, 'Chrome.*')
AND REGEXP(product.version, '^40\\.0\\.(2194|220).*')
AND crash.Reason!='EXCEPTION_BREAKPOINT'
AND REGEXP(CrashedStackTrace.StackFrame.FunctionName, '^blink::XMLHttpRequest::handleRequestError\\(')
OMIT RECORD IF SUM(REGEXP(CrashedStackTrace.StackFrame.FunctionName, '^(?:WTF::fastMalloc|WTF::StringImpl::operator new|WTF::DefaultAllocator::backingAllocate)\\(')) != 0
Project Member

Comment 24 by ClusterFuzz, Jan 23 2015

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.

Comment 25 by tkent@chromium.org, Nov 27 2015

Labels: -Cr-Blink-XHR Cr-Blink-Network-XHR
Project Member

Comment 26 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 27 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