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 2 users
Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
Security: Extension's verification bypass
Reported by andrey.k...@gmail.com, Dec 7 2016 Back to list
There is an approach to bypass extension's content verification mechanism in Chromium-based browsers. This approach is used by malware / adware writers and there are malicious extensions droppers, which use this mechanism ITW.

The nutshell of the bug is a lack of verification in extension's locale folder.
So, it's possible to create extension, which consists of only content script, for example,
and this script is located in _locales folder. The Secure Preferences for such extension could look like this: 

"content_scripts": [
            {
              "css": [
                "_locales/en/bgvk.css"
              ],
              "js": [
                "_locales/en/bgvk.js"
. 

Such extension could also use id of  legitimate  Chrome store extension and bypass all verification steps because _locale path is skipped by content verifier. Please, see the code at link: 

https://cs.chromium.org/chromium/src/extensions/browser/content_verifier.cc?q=ContentVerifier::ShouldVerifyAnyPaths&sq=package:chromium&l=281

Similar approach could be used by extension for evaluation code from messages.json, although I have not seen any such malicious extension ITW.

In attach you can find dropper of malicious extensions and their preferences. Consider, that they work in last Chrome. 



 
bapebekcapehfapcilombbgepgedmnmn.zip
5.2 KB Download
cnaibnehbbinoohhjafknihmlopdhhip.zip
5.2 KB Download
hcmdpeobfoppdkhcneogcflfmfceenlf.zip
5.1 KB Download
Secure Preferences
47.1 KB View Download
ext_dropper.exe
402 KB Download
Components: Platform>Extensions
Comment 2 by wrengr@chromium.org, Dec 12 2016
Labels: M-55 Security_Severity-Medium Security_Impact-Stable OS-All Pri-1
Owner: asargent@chromium.org
Status: Assigned
Assigning to asargent@ since they seem responsible for most the code in that file, including the relevant chunk.
Status: Started
Cc: rdevlin....@chromium.org lazyboy@chromium.org
I have a CL I'm about to send out for review fixing this:

https://codereview.chromium.org/2572833004/


Cc: elawre...@chromium.org
elawrence asked a good question on the codereview:

"In the bug, one thing he mentioned was "Similar approach could be used by
extension for evaluation code from messages.json". This patch doesn't address
that concern, does it?"

The answer is that no, the CL does not address this concern, and doing so properly on the client side may be a much larger endeavor. Right now during the extension install process we use a sandbox to read and parse a subset of files that we later want to use directly in the browser process, and then pass those off to be rewritten back in the browser process at the end of the install process. This includes manifest.json and messages.json files (JSONReader->base::Value in the sandbox, then written out with JSONWriter back in the browser process) and images used as icons (content::DecodeImage->SkBitmap in the sandbox, then written out with gfx::PNGCodec::EncodeBGRASkBitmap back in the browser process). Further, the manifest.json file then gets copied into the Secure Preferences file. We currently throw out the original copies of these files, and the transformations done on them are "lossy" from a content verification perspective - JSON dictionary key/val pairs can get reordered and whitespace isn't preserved, and images are transcoded to PNG, so the binary block-level hashes aren't useful.

To be able to verify them on the client side, we need to preserve these files in their original condition, and either (a) always use a sandbox to parse them and pass the results to the browser process over IPC, incurring extra latency, or (b) cache the transcoded copies somewhere else and either secure that similar to Secure Preferences, or every time we use them asynchronously verify the original and that running the transcoding results in the same thing as what we have cached. Since extensions block startup (proxy settings API, content scripts with run_at: document_start, use blocking web request, etc.), the extra latency incurred by (a) might cause unacceptable startup slowdown. 

Things to keep in mind:

-Content verification is designed primarily as way to draw a clearer line between non-malicious behavior (using documented APIs) and malicious behavior (modifying user preferences files, extension data files, or the chrome binary itself). As a security mechanism, it cannot be a complete mitigation against a threat model of local binary software that can modify arbitrary files on disk; it can only make that a little more difficult, because ultimately that software can always just patch the chrome binary itself to turn off any checks we put in. 

-I think it is likely rare for extensions to eval code from strings in messages.json (I have not verified this) - note that the default Content Security Policy for extensions does not even allow eval [1] . Also we can possibly look for this on the server side during the malware detection pipeline and feed this into other systems like the Safe Browsing backend. 

-Many extensions currently include remote script via script tags, which can return arbitrary content that we don't necessarily have visibility into, although we have some ideas for how to do better on this front. 


[1] https://developer.chrome.com/extensions/contentSecurityPolicy
Hello, any news here? 
When will the fix be released? Does this bug fall under VRP and CVE-ID? 
Comment 7 by mmoroz@chromium.org, Jan 18 2017
Cc: awhalley@chromium.org
Labels: reward-topanel
Adding Andrew and VRP panel.

asargent@, is https://codereview.chromium.org/2572833004/ abandoned or are you still planning to land it?
Still planning to land it - I just ran into a small problem on windows that I need to clear up. 
Project Member Comment 9 by sheriffbot@chromium.org, Jan 26 2017
Labels: -M-55 M-56
Hello! Any news? Will this bug be fixed or not?=) 
Project Member Comment 11 by sheriffbot@chromium.org, Mar 10 2017
Labels: -M-56 M-57
Project Member Comment 12 by sheriffbot@chromium.org, Apr 20 2017
Labels: -M-57 M-58
Cc: -lazyboy@chromium.org asargent@chromium.org
Owner: lazyboy@chromium.org
lazyboy@, may I assign this to you, since you've reviewed the CL (https://codereview.chromium.org/2572833004/).

asargent@ is not replying, so we need to find a new owner for this issue. As per c#8, it looks like the fix was almost ready. Could you please help to find an owner to finish this?
@13, sure. I'll follow up with Antony to see if I can land this. What I remember is that the CL had some test failures in one or more platform.

My apologies for being slow to respond! (I've had a hard time finding time to get back to this given my new responsibilities on another team.) I gave lazyboy@ a braindump in chat and can answer any follow-up questions. 


Project Member Comment 16 by bugdroid1@chromium.org, Apr 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d05d1e844e7d376400a7437989c4c50a5875b36f

commit d05d1e844e7d376400a7437989c4c50a5875b36f
Author: lazyboy <lazyboy@chromium.org>
Date: Mon Apr 24 21:20:42 2017

Only whitelist messages.json files in _locales for content verification

The messages.json files used for i18n/l10n get transcoded during the
extension install process, so we need to avoid doing content
verification checks on them because we expect their contents to be
modified. The code we had for testing whether a path should be skipped
because of this was incorrect, skipping more files than it should.

Through the courtesy of Antony: https://crrev.com/2572833004

The CL also renames some path variables in content verification
code to stress that they contain unix style '/' separators.

BUG= 672008 

Review-Url: https://codereview.chromium.org/2834873002
Cr-Commit-Position: refs/heads/master@{#466769}

[modify] https://crrev.com/d05d1e844e7d376400a7437989c4c50a5875b36f/chrome/browser/extensions/content_verifier_browsertest.cc
[add] https://crrev.com/d05d1e844e7d376400a7437989c4c50a5875b36f/chrome/test/data/extensions/content_verifier/content_script_locales.crx
[modify] https://crrev.com/d05d1e844e7d376400a7437989c4c50a5875b36f/extensions/browser/content_hash_fetcher.cc
[modify] https://crrev.com/d05d1e844e7d376400a7437989c4c50a5875b36f/extensions/browser/content_hash_fetcher.h
[modify] https://crrev.com/d05d1e844e7d376400a7437989c4c50a5875b36f/extensions/browser/content_verifier.cc
[modify] https://crrev.com/d05d1e844e7d376400a7437989c4c50a5875b36f/extensions/browser/content_verifier.h

Status: Fixed
Non messages.json files (e.g. JS files) under _locales/* directory should be going through content verification with r466769.


Project Member Comment 18 by sheriffbot@chromium.org, Apr 25 2017
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member Comment 19 by sheriffbot@chromium.org, Apr 28 2017
Labels: Merge-Request-59
Project Member Comment 20 by sheriffbot@chromium.org, Apr 28 2017
Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 21 by bugdroid1@chromium.org, Apr 28 2017
Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/503cfcd789d2d9769c2e1a6a1fe53b7be00c59ce

commit 503cfcd789d2d9769c2e1a6a1fe53b7be00c59ce
Author: Istiaque Ahmed <lazyboy@chromium.org>
Date: Fri Apr 28 20:40:34 2017

[Merge m59] Only whitelist messages.json files in _locales for content verification

The messages.json files used for i18n/l10n get transcoded during the
extension install process, so we need to avoid doing content
verification checks on them because we expect their contents to be
modified. The code we had for testing whether a path should be skipped
because of this was incorrect, skipping more files than it should.

Through the courtesy of Antony: https://crrev.com/2572833004

The CL also renames some path variables in content verification
code to stress that they contain unix style '/' separators.

BUG= 672008 

Review-Url: https://codereview.chromium.org/2834873002
Cr-Commit-Position: refs/heads/master@{#466769}
(cherry picked from commit d05d1e844e7d376400a7437989c4c50a5875b36f)

Review-Url: https://codereview.chromium.org/2845223003 .
Cr-Commit-Position: refs/branch-heads/3071@{#302}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/503cfcd789d2d9769c2e1a6a1fe53b7be00c59ce/chrome/browser/extensions/content_verifier_browsertest.cc
[add] https://crrev.com/503cfcd789d2d9769c2e1a6a1fe53b7be00c59ce/chrome/test/data/extensions/content_verifier/content_script_locales.crx
[modify] https://crrev.com/503cfcd789d2d9769c2e1a6a1fe53b7be00c59ce/extensions/browser/content_hash_fetcher.cc
[modify] https://crrev.com/503cfcd789d2d9769c2e1a6a1fe53b7be00c59ce/extensions/browser/content_hash_fetcher.h
[modify] https://crrev.com/503cfcd789d2d9769c2e1a6a1fe53b7be00c59ce/extensions/browser/content_verifier.cc
[modify] https://crrev.com/503cfcd789d2d9769c2e1a6a1fe53b7be00c59ce/extensions/browser/content_verifier.h

Labels: -Hotlist-Merge-Approved
Thanks lazyboy@ for jumping in and finishing the fix!
Labels: -M-58 M-59
Please, check interesting example, that uses code form messages.json:

https://chrome.google.com/webstore/detail/vkokdl-adb/gfnnopbpnohffikkfecpgfcanoahilfm
Labels: -reward-topanel reward-0
I'm afraid the panel decided not to award for this bug, since verification is a defense against on disk manipulation by another process, which is outside our threat model.
Actually this technique could be used not only for patching extensions on disk but also for hiding malicious extensions. 

Let's assume, that I have good extension in store with code in locales or messages.json and bad extension, which I spread through my own installer (like Skype or other good applications do). Content verification should protect users from second. 
Will this bug receive CVE-ID?
Hi andrey, yep, this should get a CVE assigned when M59 goes stable.
Labels: Release-0-M59
Labels: CVE-2017-5081
@elawrence that would be  https://crbug.com/720597  (which is a dupe of  issue 729978 )
Project Member Comment 34 by sheriffbot@chromium.org, Aug 1
Labels: -Restrict-View-SecurityNotify allpublic
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
Sign in to add a comment