Project: chromium Issues People Development process History Sign in
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
Status: Fixed
Owner:
Closed: Oct 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security



Sign in to add a comment
Do Not Cache Resources Retrieved Via Broken HTTPS in AppCache Or Service Worker
Reported by jiayaoqi...@gmail.com, Sep 13 2014 Back to list
VULNERABILITY DETAILS

For the site over HTTPS with an invalid certificate (e.g., self-signed and domain mismatch), Chrome may cache the resources (e.g., pages) over broken HTTPS in HTML5 AppCache, if the site contains AppCache manifest. If the user is under an MITM attack in a public area (e.g., airport), the attacker can replace the site's page with a malicious one that contains a long-lived (e.g., one year) AppCache manifest, once the user clicks through an SSL warning. When the user returns to a safe place without under MITM attacks, and visits the same site, Chrome will directly load the malicious page in AppCache instead of issuing new requests until the long-lived manifest expires. 

VERSION
Chrome Version: [37.0.2062.120] + [stable] (previous versions may have the same problem.)
Operating System: [OS X 10.9.3]

REPRODUCTION CASE

0) Suppose the targeted site is https://yahoo.com, the attacker hosts a malicious server and site, and the victim is under a MITM attack. The attacker can utilize a host of well-known MITM techniques (e.g., ARP poisoning and DNS pharming attacks) to re-route all the traffic of the victim to himself. In our experiment, we use mitmproxy to simulate the attack.
1) When the user visits https://yahoo.com, the attacker intercepts the connection;
2) Once the user clicks through the SSL warning raised by Chrome, the attacker can substitute the malicious page for the original one. The malicious one contains a long-lived AppCache manifest;
3) Chrome caches the malicious copy in AppCache for a long time. When the user visits https://yahoo.com again in a safe place, his Chrome will directly load the malicious copy from AppCache.

To simply demonstrate the caching scenario: 
1) Download the attached test.html and test.appcache;
2) Host the two files on the local server that enables HTTPS with self-signed certificate;
3) Using Chrome visit test.html and click through the SSL warnings;
4) Check "chrome://appcache-internals/", and Chrome will show that test.html is cached in AppCache.

In all, Chrome caches hijacked resources over HTTPS in AppCache in the MITM scenario.
The attached picture "yahoo1" is the malicious copy in AppCache, and the "appcache-internals" is the screenshot to show that Chrome caches replaces page over broken HTTPS in AppCache.

At present, Chrome does not cache resources over broken HTTPS connections, which is desirable for security. However, HTML5 AppCache resources over broken HTTPS connections should not be cached as well, and such security restriction has already been implemented in Safari.
 
test.html
102 bytes View Download
appcache-internals.png
104 KB View Download
test.appcache
55 bytes Download
yahoo1.png
18.8 KB View Download
Comment 1 by tsepez@chromium.org, Sep 16 2014
Labels: Security_Impact-Stable Security_Severity-Medium M-38 Pri-2
Owner: palmer@chromium.org
Status: Assigned
@palmer - care to take a stab at this one? Thanks.
Project Member Comment 2 by clusterf...@chromium.org, Sep 16 2014
Labels: -Pri-2 Pri-1
Comment 3 by palmer@chromium.org, Sep 16 2014
Cc: michaeln@chromium.org
Labels: Cr-Blink-Storage-AppCache
michaeln: Any pointers on where in the code I should start looking?
Comment 4 by palmer@chromium.org, Sep 16 2014
Summary: Do Not Cache Resources Retrieved Via Broken HTTPS in AppCache (was: Security: Cache Resources over Broken HTTPS in AppCache)
appcache_update_job.cc
Look at it's URLFetcher class in particular.
Cc: dominicc@chromium.org jsb...@chromium.org
We need to do something similar for service workers.
Labels: Cr-Blink-ServiceWorker
Comment 9 by palmer@chromium.org, Sep 16 2014
Cc: slightlyoff@chromium.org jakearchibald@chromium.org
Summary: Do Not Cache Resources Retrieved Via Broken HTTPS in AppCache Or Service Worker (was: Do Not Cache Resources Retrieved Via Broken HTTPS in AppCache)
Status: Started
Hi, michaeln & palmer,

Do you double-check this security bug on other versions of Chrome? I've tested on 35, 36 and 37, but not sure about other versions. 
BTW, I have a suggestion to fix this bug. You can simply modify the source code to let Chrome not cache the manifest over broken HTTPS in AppCache. Thus Chrome will not store other resources listed in the manifest. Thanks.
We only support the latest stable version of Chrome. The fix for this bug will go into Chrome 38.
Note that according to https://code.google.com/p/chromium/issues/detail?id=78859, this bug should not be happening?! So, something very odd is going on.
Cc: rsleevi@chromium.org
I'm still having a really hard time find my way around the maze of indirect function calls. I think it might be best if someone else takes this bug. Also, maybe rsleevi has some clues.
Cc: davidben@chromium.org
Owner: michaeln@chromium.org
David's been looking carefully at a lot of stuff, and probably can dig in further.

As for where this happens presently in the //net stack, it's https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_cache_transaction.cc&l=2628

The magic you're looking for is:
if (net::IsCertStatusError(response_.ssl_info.cert_status))

I honestly can't follow all the crazy logic in app cache / service worker either, but it seems like the correct thing to do is simply not allow an SW/AC to be installed if the document ssl_info meets the above criteria.

Pushing to Michael for app cache. I'm pretty sure the critical path is https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/appcache/appcache_update_job.cc&rcl=1411534708&l=504

> net::IsCertStatusError(response_.ssl_info.cert_status)

Yup, that looks like the 'magic' that was missing.
We should probably reject any resource with a cert error, URLFetcher probably can error out early in...

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/appcache/appcache_update_job.cc&rcl=1411534708&l=163
Project Member Comment 19 by clusterf...@chromium.org, Oct 10 2014
Labels: Nag
michaeln@: 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
Project Member Comment 21 by clusterf...@chromium.org, Oct 18 2014
michaeln@: 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
Status: Fixed
Project Member Comment 23 by clusterf...@chromium.org, Oct 20 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
Cc: horo@chromium.org
CC horo.  Issue 425396  sounds related?
Comment 25 by horo@chromium.org, Oct 21 2014
Yes  Issue 425396  is related this issue.
I think we should add net::IsCertStatusError() check in ServiceWorkerWriteToCacheJob::OnResponseStarted().
Project Member Comment 26 by bugdroid1@chromium.org, Oct 22 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a8ac964e0cf580c4e72028f81f1cda335436659d

commit a8ac964e0cf580c4e72028f81f1cda335436659d
Author: horo <horo@chromium.org>
Date: Wed Oct 22 07:13:11 2014

[ServiceWorker] Don't register ServiceWorker with an invalid HTTPS certificate.

BUG= 425396 , 414026 

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

Cr-Commit-Position: refs/heads/master@{#300644}

[modify] https://chromium.googlesource.com/chromium/src.git/+/a8ac964e0cf580c4e72028f81f1cda335436659d/content/browser/service_worker/service_worker_write_to_cache_job.cc

horo found more magic, net::HttpNetworkSession::Params.ignore_certificate_errors should be looked at too
Cc: falken@chromium.org nhiroki@chromium.org
Labels: -Nag -Merge-Triage -M-39 Release-0-M40
Let it roll into M40
Labels: reward-topanel
Project Member Comment 32 by bugdroid1@chromium.org, Nov 14 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c65cc9c7b1afe7f53bb18707bc711dc32ddffad0

commit c65cc9c7b1afe7f53bb18707bc711dc32ddffad0
Author: michaeln <michaeln@chromium.org>
Date: Fri Nov 14 01:40:39 2014

Do not AppCache responses with SSL cert errors unless running with the --ignore-certificate-errors flag.

BUG= 414026 

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

Cr-Commit-Position: refs/heads/master@{#304140}

[modify] https://chromium.googlesource.com/chromium/src.git/+/c65cc9c7b1afe7f53bb18707bc711dc32ddffad0/content/browser/appcache/appcache_update_job.cc

Cc: sgu...@chromium.org benm@chromium.org jarmour@chromium.org
Labels: -reward-topanel reward-unpaid CVE-2014-7948 reward-500
Congratulations - $500 for this report! Notes from panel: "User would have to click through bad HTTPS to trigger,. reducing the reward amount to $500".

We've credited you in our release notes as "jiayaoqijia" - let me know if you want to use a different name/handle.

Someone should be in touch within a few weeks to collect payment details.
Hi, tim,

Thanks for the reward. The name can be changed to Yaoqi Jia. Thanks.
Project Member Comment 36 by clusterf...@chromium.org, Jan 27 2015
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Labels: -reward-unpaid reward-inprocess
Updated your name for credit: http://googlechromereleases.blogspot.com/2015/01/stable-update.html

You should also receive an email shortly regarding payment.
Hi, tim,

Got it. Thanks. :)
Labels: -reward-inprocess
Processing via our e-payment system can take up to two weeks, but the reward should be on its way to you. Thanks again for your help!
Project Member Comment 41 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 42 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