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

Issue metadata

Status: Fixed
Owner:
OOO till Aug 20
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Devtools old remote frontend allows running privileged scripts via overwriting localStorage settings

Reported by gregory....@gmail.com, Jun 7 2016

Issue description

VULNERABILITY DETAILS
Specially crafted input to whitelisted URLs on Devtools remote frontend, allows an attacker to manipulate Devtool settings stored in localStorage. One possible injection point that allows attacker to run arbitrary javascript code is via watchExpression that runs in the context of the website when DevTools is launched. Other injection points of arbitrary javascript expression is through conditional breakpoints on specific pages.

VERSION
Chrome Version: 51.0.2704.84 m stable
Operating System: All

REPRODUCTION CASE
1. Copy paste "chrome-devtools://devtools/remote/serve_rev/@180642/devtools.html?settings={%22watchExpressions%22:%22[\%22alert(document.domain)\%22]%22}" to omnibar
2. Press CTRL-SHIFT-I [in windows] to open DevTools, Switch to Sources panel ( default panel setting can possibly be controlled by injecting the 'lastActivePanel' setting )
3. Observe alert 'devtools'

An example of it running privileged javascript code [to access filesystem] is attached. It basically injects an expression that checks for location.host==devtools, and access filesystem using DevToolsAPI.sendMessageToEmbedder("loadNetworkResource",...) calls.
[you can observe the 1st file on filesystem displayed on webpage]


 
Devtools-Crafted-URI1.txt
1.4 KB View Download
Thanks for the report.  Can you produce a demonstration that doesn't require pasting a URL into the omnibox?  The same alert() can be generating with a pasted javascript:alert() URL.
This issue has some similarity to 571121 (where it is discussed in length). One of the other attack vector mentioned there is by using a malicious extension (with zero permission) that can open chrome-devtools:// URLs.

The main security issue is the ability to change Devtools' settings stored in localStorage by visiting the URL.

The maximum impact is demonstrated by execution privilege script (in Step#2) that gets access to filesystem (attached Devtools.Crafted-URL1.txt in orig report). If the inspected page is a regular webpage, it can read cookie-data etc. and send it to the attacker.

Cc: taviso@google.com rdevlin....@chromium.org dgozman@chromium.org jsc...@chromium.org mea...@chromium.org
Components: Platform>DevTools>Platform
Labels: Security_Severity-Medium Security_Impact-Stable
Owner: dgozman@chromium.org
Status: Assigned (was: Unconfirmed)
I repro'd the example on M51 stable.  Maybe these chrome-devtools:// links write to settings should require the referrer's scheme to be the same.

dgozman@ -- Can you find an appropriate owner?
Grabbing cc list from  http://crbug.com/571121 


Project Member

Comment 5 by sheriffbot@chromium.org, Jun 10 2016

Labels: Pri-1
Labels: M-52
Project Member

Comment 7 by sheriffbot@chromium.org, Jun 22 2016

dgozman: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
FYI - One more Attack vector (besides malicious extension; copy-pasting URLs) is by sending a crafted chrome-devtools link via "Google Tone" extension. The extension allows sending URLs of any schemes to nearby machines.
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 6 2016

dgozman: Uh oh! This issue still open and hasn't been updated in the last 28 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by ta...@google.com, Jul 13 2016

Labels: OS-All

Comment 11 by ta...@google.com, Jul 13 2016

Labels: -OS-All OS-Chrome

Comment 12 by ta...@google.com, Jul 13 2016

Labels: -OS-Chrome OS-All

Comment 13 by taviso@google.com, Jul 13 2016

Can't we just set a minimal CL for whatever appengine code is serving that?

That seems like a quick easy fix.
Cc: pfeldman@chromium.org
I don't think this is a problem. It requires user to 1) open url which cannot be linked to and 2) open DevTools for that page manually.

It's equivalent to opening DevTools for DevTools and executing any code in console, which anyone can do if they want.

Any thoughts from security experts?

Comment 15 by taviso@google.com, Jul 23 2016

The problem is that an extension with no permissions can navigate to that URL.

This is a variant of  bug 571121 , afaik the same discussion applies.
IIUC, this one requires the additional step of opening DevTools after navigation to the specific url (see step 2 in original report).

Comment 17 by taviso@google.com, Jul 23 2016

Ah, I missed that - that does make it very low severity!

But...it does seem prudent to have a way to prevent loading earlier revisions in case a bug does surface that is exploitable from just URL parameters (or with more realistic interaction)?
IMHO, at the very least, we wouldn't want a malicious extension overwriting DevTools' localStorage settings. For it to cause any sort of damage, the DevTools needs to opened to inspect a page at some point in time. 

Comment 19 by taviso@google.com, Jul 23 2016

Oh I see, so if at anypoint in future you open devtools, it still fires?

Well, that does seem bad?
The overwritten settings is persisted. You can set malicious watch-expression, or conditional (with malicious expression) breakpoints on various webpages, that can fire when devtools/sources-panel is opened to inspect that page. 
Project Member

Comment 21 by bugdroid1@chromium.org, Jul 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5e6098dc2e984befc836f482845137245fa04e2

commit d5e6098dc2e984befc836f482845137245fa04e2
Author: dgozman <dgozman@chromium.org>
Date: Tue Jul 26 01:11:02 2016

[DevTools] Do not use "settings" query param.

Implemented the same functionality on embedder side.
Also, sanitizing "settings" query param for remote frontends.

BUG= 618037 

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

[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/chrome/browser/devtools/devtools_window.h
[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/content/shell/browser/layout_test/blink_test_controller.cc
[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/content/shell/browser/layout_test/layout_test_devtools_frontend.cc
[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/content/shell/browser/layout_test/layout_test_devtools_frontend.h
[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/content/shell/browser/shell_devtools_frontend.cc
[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/content/shell/browser/shell_devtools_frontend.h
[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/third_party/WebKit/Source/devtools/front_end/devtools.js
[modify] https://crrev.com/d5e6098dc2e984befc836f482845137245fa04e2/third_party/WebKit/Source/devtools/front_end/main/Main.js

I think we can merge the fix to M53 after it bakes in canary for a while.
Hi dgozman@ - this hasn't been in any of the queries for M53 consideration since it's not yet marked as fixed.  Are you expecting any more code changes?
Labels: -M-52 M-53
I prefer to bake the fix in canary for a little longer. It's one of the non-obvious areas, where we'd better be more careful. We still have plenty of time for M53, and I don't want to merge this to M52.
Labels: Merge-Request-53
Requesting to merge https://chromium.googlesource.com/chromium/src.git/+/d5e6098dc2e984befc836f482845137245fa04e2 to M53.

Comment 26 by dimu@chromium.org, Aug 8 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 27 by bugdroid1@chromium.org, Aug 8 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/956afd761251b3e5897165a5ea5434e84f0882fb

commit 956afd761251b3e5897165a5ea5434e84f0882fb
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Mon Aug 08 17:13:39 2016

Merge to 2785 "[DevTools] Do not use "settings" query param."
> [DevTools] Do not use "settings" query param.
>
> Implemented the same functionality on embedder side.
> Also, sanitizing "settings" query param for remote frontends.
>
> BUG= 618037 
>
> Review-Url: https://codereview.chromium.org/2177983004
> Cr-Commit-Position: refs/heads/master@{#407666}
(cherry picked from commit d5e6098dc2e984befc836f482845137245fa04e2)
TBR=pfeldman

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

Cr-Commit-Position: refs/branch-heads/2785@{#529}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/chrome/browser/devtools/devtools_window.h
[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/content/shell/browser/layout_test/blink_test_controller.cc
[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/content/shell/browser/layout_test/layout_test_devtools_frontend.cc
[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/content/shell/browser/layout_test/layout_test_devtools_frontend.h
[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/content/shell/browser/shell_devtools_frontend.cc
[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/content/shell/browser/shell_devtools_frontend.h
[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/third_party/WebKit/Source/devtools/front_end/devtools.js
[modify] https://crrev.com/956afd761251b3e5897165a5ea5434e84f0882fb/third_party/WebKit/Source/devtools/front_end/main/Main.js

Status: Fixed (was: Assigned)
Project Member

Comment 29 by sheriffbot@chromium.org, Aug 9 2016

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -Hotlist-Merge-Approved
Labels: Release-0-M53
Labels: reward-topanel
Labels: reward-topanel
Labels: -reward-topanel reward-unpaid reward-1000
The panel has decided to award $1,000 for this bug, noting that it's more heavily mitigated than other related reports.  Many thanks as ever!
Thanks :)
Labels: -reward-unpaid reward-inprocess
Labels: CVE-2016-5165
Project Member

Comment 39 by sheriffbot@chromium.org, Nov 15 2016

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

Comment 40 by lon...@gmail.com, Nov 25 2016

Hi dgozman, your patch only filter the input from PC chrome, how to protect the mobile version?
Labels: CVE_description-submitted

Sign in to add a comment