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

Issue 108648 link

Starred by 1 user

Issue metadata

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

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Security: Malicious extension could avoid being blacklisted via extension blacklist

Reported by adblockf...@gmail.com, Dec 26 2011

Issue description

This template is ONLY for reporting security bugs. Please use a different
template for other types of bug reports.

Please see the following link for instructions on filing security bugs:
http://www.chromium.org/Home/chromium-security/reporting-security-bugs


VULNERABILITY DETAILS
Please provide a brief explanation of the security issue.

webRequest.onBeforeRequest can intercept calls to  http://www.gstatic.com/chrome/extensions/blacklist/l_0_0_0_7.txt.  Thus if an extension went rogue, Google could add the extension to the blacklist but the extension could prevent Chrome from receiving the blacklist update.

If this blacklist is for buggy extensions rather than malicious extensions, then this is not a security bug.

VERSION
Chrome Version: 17.0.963.12 dev-m
Operating System: Any

REPRODUCTION CASE
Please include a demonstration of the security bug, such as an attached
HTML or binary file that reproduces the bug when loaded in Chrome. PLEASE
make the file as small as possible and remove any content not required to
demonstrate the bug.

Put this in a background script in an extension:

chrome.webRequest.onBeforeRequest.addListener(function(details) {
 var block = (details.url.indexOf("blacklist") !== -1);
 console.log(details.url, block);
 return { cancel: block };
}, {urls: ["http://*/*"]}, ["blocking"]);

and you should see http://www.gstatic.com/chrome/extensions/blacklist/l_0_0_0_7.txt true
appear in the background console logs.

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: [tab, browser, etc.]
Crash State: [see link above: stack trace, registers, exception record]
Client ID (if relevant): [see link above]
 
Cc: erikkay@chromium.org
Labels: -Pri-0 -Area-Undefined Pri-1 Area-Internals Feature-Extensions SecSeverity-Medium OS-All Mstone-16 SecImpacts-Stable SecImpacts-Beta
Owner: aa@chromium.org
Status: Assigned
Aaron, Erik, can you please suggest an owner for this.

Comment 2 by aa@chromium.org, Dec 26 2011

Cc: battre@chromium.org mpcomplete@chromium.org
Owner: battre@chromium.org
Dominic, Matt, I don't think that webRequest should see system requests.

Comment 3 by jsc...@chromium.org, Dec 27 2011

Labels: -SecImpacts-Stable
This isn't enabled by default on stable. Also (to reiterate @aa's statement) based on previous discussions the WebRequest API definitely should not be able to block these requests.

@adblockforchrome - Just to verify, did you browse to the URL manually or did you wait for the blacklist check?

Comment 4 by jsc...@chromium.org, Dec 27 2011

Labels: -SecSeverity-Medium SecSeverity-Low
Hopefully we also remembered to block https://chrome.google.com/ from being messed with by webRequest ?

Comment 6 by battre@chromium.org, Dec 27 2011

Status: Started
This is slightly more complicated to fix cleanly than anticipated because fetching the blacklist is not a system request. System requests are already hidden from WebRequest API extensions. I guess all we can do for now is blacklisting some URLs. I'll prepare a CL.

Comment 7 by battre@chromium.org, Dec 27 2011

@scarybeasts: can you give me more information on https://chrome.google.com/? This seems to be a redirect to https://www.google.com/chrome/, just blacklisting https://chrome.google.com/ is therefore insufficient.

What do we want to protect here and why?

Comment 8 by battre@chromium.org, Dec 27 2011

A CL is available for review here: http://codereview.chromium.org/8952021/

Please let me know if you don't have access (I have marked the CL as private).
@3: I did not browse to the URL manually.  I noticed this behavior in my logging of details.tabId===-1 onBeforeRequest events.

@7: I assume you want to protect pages in the Web Store from modification.  I can currently make the Web Store "unreachable" by blocking all requests that match /webstore/ (see screenshot).  tabs.executeScript still fails and my content scripts do not run.
webstore.png
50.1 KB View Download
Yes, it should not be possible to modify anything under https://chrome.google.com/ or https://www.google.com/chrome/

Also, I just noticed tat the blacklist is downloaded over HTTP. That would allow a man-in-the-middle to arbitrarily blacklist extensions. So, I'll file a separate bug on that one.
Cc: bauerb@chromium.org
Cc: asargent@chromium.org
@battre - what do you mean when you say that the blacklist "is not a system request"?  It uses the extension autoupdate mechanism which definitely should be a system request.  We don't want extensions intercepting and modifying autoupdate requests or downloads.
My understanding of extension_updater.cc is that it uses the request context of the profile.
During the design discussions I thought we'd agreed that webrequest would not apply to system update requests (eg. safebrowsing, extension blacklist), internal requests (eg. chrome URLS) or anything within the webstore extent. Are there any areas where we're diverging from that?
I think we should just switch extension updates to be system requests (there wasn't really such a concept at the time the code was written, and in fact we go out of our way to avoid sending/storing cookies for instance so I don't think we have any reason to have them tied to a profile). 

Also, @10 - the blacklist manifest is fetched over https and includes a sha256 hash - when we fetch the blacklist content over http we verify that the content's hash matches that. See ExtensionUpdater::ProcessBlacklist. 
Justin:
The WebRequest API sees only requests, if they originate from a Profile (not system context), and
- are not sent from another extension, and
- have one of the following schemes: about, file, ftp, http, https, chrome-extension

There is one corner case about synchronous XHRs from the same extension that is not so important here: A blocking listener does not receive synchronous XHRs from the same extension to prevent deadlocks.

The CL referenced above will prevent an extension to see requests
- to the extension gallery (https://chrome.google.com/webstore/*)
- *://*.google.com/chrome
- http://www.gstatic.com/chrome/extensions/blacklist/*, https://www.gstatic.com/chrome/extensions/blacklist/*

Unfortunately, URLRequests are not annotated with their purpose and origin. This makes it difficult to know whether you have caught all relevant cases.
A "system request" is one that uses the system request context, which is separate from that of the profile. Are we sure that extension autoupdates are system-level? Intuitively they seem like they are tied to a profile (the profile that has the extension installed).

One of the goals of the webRequest API was to support an extension like Tor, which needs to ensure all requests made via the profile have certain privacy protections. When we don't allow this (like on the gallery, where we disallow content scripts), they'd like to block the page load and warn the user (see discussion on  issue 33505  ). The referenced CL would prevent them from doing this.

Is there a reason to disallow webRequest access to the gallery?
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 4 2012

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

------------------------------------------------------------------------
r116258 | battre@chromium.org | Tue Jan 03 17:11:28 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/extensions/extension_constants.h?r1=116258&r2=116257&pathrev=116258
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_webrequest_api.cc?r1=116258&r2=116257&pathrev=116258
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/extensions/extension_constants.cc?r1=116258&r2=116257&pathrev=116258
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_updater.cc?r1=116258&r2=116257&pathrev=116258

Hide downloads of extensions blacklist from web request API.

This CL prevents that extensions using the web request API can prevent Chrome from updating its extensions blacklist.

R=mpcomplete@chromium.org
BUG= 108648 
TEST=no


Review URL: http://codereview.chromium.org/8952021
------------------------------------------------------------------------
Cc: kerz@chromium.org
Labels: -Mstone-16 Mstone-17 Merge-Requested
+kerz for merge request to M17 as this security relevant

Comment 20 by k...@google.com, Jan 7 2012

Labels: -Merge-Requested Merge-Approved
Project Member

Comment 21 by bugdroid1@chromium.org, Jan 9 2012

Labels: -merge-approved merge-merged-963
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=116902

------------------------------------------------------------------------
r116902 | battre@chromium.org | Mon Jan 09 11:53:35 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/963/src/chrome/common/extensions/extension_constants.h?r1=116902&r2=116901&pathrev=116902
 M http://src.chromium.org/viewvc/chrome/branches/963/src/chrome/browser/extensions/extension_webrequest_api.cc?r1=116902&r2=116901&pathrev=116902
 M http://src.chromium.org/viewvc/chrome/branches/963/src/chrome/common/extensions/extension_constants.cc?r1=116902&r2=116901&pathrev=116902
 M http://src.chromium.org/viewvc/chrome/branches/963/src/chrome/browser/extensions/extension_updater.cc?r1=116902&r2=116901&pathrev=116902

Merge 116258 - Hide downloads of extensions blacklist from web request API.

This CL prevents that extensions using the web request API can prevent Chrome from updating its extensions blacklist.

R=mpcomplete@chromium.org
BUG= 108648 
TEST=no


Review URL: http://codereview.chromium.org/8952021

TBR=battre@chromium.org
Review URL: http://codereview.chromium.org/9152010
------------------------------------------------------------------------
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 10 2012

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

------------------------------------------------------------------------
r116960 | battre@chromium.org | Mon Jan 09 15:59:54 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_webrequest_api.cc?r1=116960&r2=116959&pathrev=116960

Use GURL::Replacements to trim query and ref from URL


BUG= 108648 
TEST=no


Review URL: http://codereview.chromium.org/9120011
------------------------------------------------------------------------
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved
Status: FixUnreleased
Can you please make sure all the pieces are merged to m18 beta 1025 branch. Is there anything else in the bug pending fixed. I see you merged some stuff to m17, but since this bug wasnt closed, it missed an entry in the release notes.
Yeah, looks like we need the extensions team to merge this to 1025.
Labels: -Mstone-17 Mstone-18 SecImpacts-Stable
Hmm, looks like this may have actually made it into M17 without us correctly crediting the reporter? We'll rectify that in the M18 release notes.

@adblockforchrome -- let us know if there's some other name you'd like us to credit you under.
@25: Credit as "Michael Gundlach" would be good.  Thanks!
The core is already in branch 1025 (stuff from http://codereview.chromium.org/8952021), the change from was deemed not security critical. Let me know if you want me to merge it back.
Labels: -Merge-Approved Merge-Merged
Ah, you're right. From the revision numbers, this is obviously in M18. Thanks! :) Sorry for the noise.
I'll update the release notes with proper credit.
Labels: CVE-2011-3049

Comment 30 by cdn@chromium.org, May 15 2012

Status: Fixed
Marking old security bugs Fixed..
Project Member

Comment 31 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 32 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Type-Security -Area-Internals -Feature-Extensions -SecSeverity-Low -Mstone-18 -SecImpacts-Beta -SecImpacts-Stable Cr-Platform-Extensions Security-Impact-Stable Security-Impact-Beta Security-Severity-Low M-18 Cr-Internals Type-Bug-Security
Project Member

Comment 33 by bugdroid1@chromium.org, Mar 13 2013

Labels: Restrict-View-EditIssue
Project Member

Comment 34 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Project Member

Comment 36 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Severity-Low Security_Severity-Low
Project Member

Comment 37 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member

Comment 38 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Beta Security_Impact-Beta
Project Member

Comment 39 by sheriffbot@chromium.org, Jun 14 2016

Labels: -security_impact-beta
Project Member

Comment 40 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 41 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