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

Issue 558589 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 0
Type: Bug-Security



Sign in to add a comment

Security: AppCacheUpdateJob UaF

Reported by gzo...@gmail.com, Nov 19 2015

Issue description

AppCacheUpdateJob::URLFetcher has a raw job_ pointer to AppCacheUpdateJob. A fetcher that outlives job causes UaF.

One check that keeps a job alive is in AppCacheUpdateJob::MaybeCompleteUpdate(): master_entries_completed_ != pending_master_entries_.size(). A new master request is initiated at AppCacheUpdateJob::StartUpdate() which inserts the master url into pending_master_entries_. If that's the first encounter of this master url, a new request is initated at AppCacheUpdateJob::AddMasterEntryToFetchList(). The master url is considered to be a first encounter when it hasn't been fetched yet, it isn't currently being fetched and it isn't scheduled to be fetched. AddMasterEntryToFetchList() checks whether the url has been fetched by calling AppCache::GetEntry(). This returns an existing entry if one has been set by AppCache::AddOrModifyEntry() in AppCacheUpdateJob::HandleMasterEntryFetchCompleted().

If a master url request returns a 404 though, the completion is counted in master_entries_completed_ but a cache entry is not created. So if another master is added with the same url, this seems like a first encounter and a new request is started. As the second request finishes, master_entries_completed_ is incremented again for the same url. But this url in only inserted once in the pending_master_entries_. This allows an attacker to get master_entries_completed_ == pending_master_entries_.size() while a request is actually still ongoing. The job is deleted and the fetcher will access it, leading to UaF.

Crashes the browser without requiring a compromised renderer. Working on a fix.

Chrome Version: 46 stable and up
Operating System: all

REPRODUCTION CASE
- Unpack appcache_poc.tar.gz
- Run ./webserver and navigate to http://localhost:8000
 
appcache_poc.tar.gz
1.1 KB Download
crash.txt
5.5 KB View Download

Comment 1 by wfh@chromium.org, Nov 19 2015

Cc: dcheng@chromium.org davidben@chromium.org
Labels: Security_Impact-Stable Security_Severity-Critical Pri-0 Cr-Blink-Storage-AppCache
Owner: michaeln@chromium.org
Status: Assigned
Michaeln this looks like an exploitable sandbox escape. I am working on verifying the repro in clusterfuzz.

Note: The reporter has confirmed that they are working on a fix, hoping to have something by tomorrow.
wow, super high quality bug report and repro case!
Project Member

Comment 3 by ClusterFuzz, Nov 19 2015

Labels: M-47

Comment 4 Deleted

Comment 5 by gzo...@gmail.com, Nov 20 2015

#2: Thank you! I'll link your CL here:

https://codereview.chromium.org/1463463003/
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 21 2015

Status: Fixed
Project Member

Comment 8 by ClusterFuzz, Nov 21 2015

Labels: -Restrict-View-SecurityTeam Merge-Triage M-48 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-Request-48 Merge-Request-47
Labels: -Merge-Triage

Comment 11 by tin...@google.com, Nov 23 2015

Labels: -Merge-Request-47 Merge-Review-47 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M47, manual review required.

Comment 12 by tin...@google.com, Nov 23 2015

Labels: -Merge-Request-48 Merge-Approved-48 Hotlist-Merge-Approved
Congrats your change is auto-approved for M48 (branch: 2564)
Labels: OS-All
Labels: -Merge-Review-47 -Hotlist-Merge-Review Merge-Approved-47
Merge approved for M47 (branch 2526)
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 24 2015

Labels: -Merge-Approved-47 merge-merged-2526
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57f7a2257395a31c4716bd6a1c2d6c9c09938c7b

commit 57f7a2257395a31c4716bd6a1c2d6c9c09938c7b
Author: Michael Nordman <michaeln@google.com>
Date: Tue Nov 24 00:30:05 2015

AppCache: fix a browser crashing bug that can happen during updates.

BUG= 558589 

Review URL: https://codereview.chromium.org/1463463003

Cr-Commit-Position: refs/heads/master@{#360967}
(cherry picked from commit e5c298b780737c53fa9aae44d6fef522931d88b0)

Review URL: https://codereview.chromium.org/1469253002 .

Cr-Commit-Position: refs/branch-heads/2526@{#468}
Cr-Branched-From: cb947c0153db0ec02a8abbcb3ca086d88bf6006f-refs/heads/master@{#352221}

[modify] http://crrev.com/57f7a2257395a31c4716bd6a1c2d6c9c09938c7b/content/browser/appcache/appcache_update_job.cc
[modify] http://crrev.com/57f7a2257395a31c4716bd6a1c2d6c9c09938c7b/content/browser/appcache/appcache_update_job.h

Labels: reward-topanel
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 24 2015

Labels: -Merge-Approved-48 merge-merged-2564
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c84775c9ffda1b267ee1eb9f1d5b1ae4de889975

commit c84775c9ffda1b267ee1eb9f1d5b1ae4de889975
Author: Michael Nordman <michaeln@google.com>
Date: Tue Nov 24 00:41:59 2015

AppCache: fix a browser crashing bug that can happen during updates.

BUG= 558589 

Review URL: https://codereview.chromium.org/1463463003

Cr-Commit-Position: refs/heads/master@{#360967}
(cherry picked from commit e5c298b780737c53fa9aae44d6fef522931d88b0)

Review URL: https://codereview.chromium.org/1471163002 .

Cr-Commit-Position: refs/branch-heads/2564@{#95}
Cr-Branched-From: 1283eca15bd9f772387f75241576cde7bdec7f54-refs/heads/master@{#359700}

[modify] http://crrev.com/c84775c9ffda1b267ee1eb9f1d5b1ae4de889975/content/browser/appcache/appcache_update_job.cc
[modify] http://crrev.com/c84775c9ffda1b267ee1eb9f1d5b1ae4de889975/content/browser/appcache/appcache_update_job.h

Project Member

Comment 18 by bugdroid1@chromium.org, Nov 24 2015

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/bling/chromium.git/+/c84775c9ffda1b267ee1eb9f1d5b1ae4de889975

commit c84775c9ffda1b267ee1eb9f1d5b1ae4de889975
Author: Michael Nordman <michaeln@google.com>
Date: Tue Nov 24 00:41:59 2015

Cc: clm@google.com
Labels: Release-0-M47
Labels: -reward-topanel reward-unpaid CVE-2015-6765 reward-10000
Congrats - $10,000 for this awesome report. I'll start the payment process shortly.

Comment 22 by gzo...@gmail.com, Dec 1 2015

Thank you :)
Labels: -reward-unpaid reward-inprocess
Labels: -reward-inprocess
Project Member

Comment 25 by ClusterFuzz, Mar 2 2016

Labels: -Restrict-View-SecurityNotify
This security bug has been closed for more than 14 weeks. Removing view restrictions.

- Your friendly Sheriffbot
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
Labels: CVE_description-submitted

Sign in to add a comment