New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Linkified URLs in DevTools are not sanitized (can open privileged URLs)

Project Member Reported by rob@robwu.nl, Dec 29 Back to list

Issue description

VULNERABILITY DETAILS
The DevTools automatically turns URLs in the console view into links. This is implemented by, with minimal validation [1] (any URL that does not start with "javascript:" is accepted).

Upon clicking such a link, the URL is passed to InspectorFrontendHost.openInNewTab [2]. The browser-side message handler (DevToolsUIBindings::OpenInNewTab) calls DefaultBindingsDelegate::OpenInNewTab [3], which opens the URL without restrictions (because the is_renderer_initiated parameter is false).

The URLs are visible by default, but it is trivial to construct a console message that:
- opens a restricted URL upon click (see above).
- is invisible (using the "%c" console formatter and a transparent color).

This enables web pages to trick users into opening privileged URLs (and abuse their vulnerabilities, e.g.  bug 618333  or  bug 797525 ).

VERSION
Chrome Version: 63.0.3239.108 (stable) + 65.0.3308.0 (canary)

REPRODUCTION CASE
1. Open poc.html
2. Open the DevTools console for that tab.
3. Click anywhere in the console (between the markers "Some fake string start" and "Some fake string end")
4. Observe that the browser quits ( because the invisible link is chrome://quit ).


[1] https://chromium.googlesource.com/chromium/src/+/10d932801bc9e23960f855f2bc4fd9f6a67a5f53/third_party/WebKit/Source/devtools/front_end/components/Linkifier.js#346
[2] https://chromium.googlesource.com/chromium/src/+/10d932801bc9e23960f855f2bc4fd9f6a67a5f53/third_party/WebKit/Source/devtools/front_end/components/Linkifier.js#601
[3] https://chromium.googlesource.com/chromium/src/+/4880930d19b442b3b7fc4d7c0c9933098151358a/chrome/browser/devtools/devtools_ui_bindings.cc#241
 
poc.html
629 bytes View Download
Interesting. It's not entirely clear what ought to happen here, insofar as the Developer Tools should be allowed to inspect privileged pages.
Labels: Security_Severity-Medium Security_Impact-Stable
Owner: dgozman@chromium.org
Status: Assigned (was: Untriaged)
dgozman@ What do you think about this? elawrence@ raises a good point.
Labeling Medium because it requires user interaction in the dev-tools.
Cc: dgozman@chromium.org
Owner: l...@chromium.org
What's the problem here? User can open a tab with restricted url from DevTools - they could just as well paste that into omnibox. We can sanitize chrome:// urls just to be consistent with other places though.

The bigger problem I see is that we linkify url which is transparent. Perhaps, it makes sense to force visible style for links. 

Erik, mind looking into both of these?
The problem here is the combination
- web pages can fully fill the console with link(s) that do not appear to be links (e.g. be completely invisible).
- The links can point to any destination that are normally not accessible from the web page.

The issues in isolation are not that useful, but together they can be used to trick web developers into opening restricted URLs (e.g. when they click on the console window with the intent to focus it).

The ability to unexpectedly open restricted URLs can be chained to other vulnerabilities (e.g.  bug 798163 ) to achieve a greater impact (e.g. read any local file and send it to a remote site).
Thanks for the report.

Invisible links:
Forcing visible style for links would help users will see what they are clicking on.  However, there are all sorts of style formatters we currently support: font-size, line-height, margin; 'visibility' is not just ensuring text color is black.  I'm thinking that links would need to ignore the style formatter entirely for it to work.  Other reports (  crbug.com/668591  ) indicate that some DevTools users use %c to color links in Console, so forcing visible style could be a significant regression.


Blocking chrome:// urls:
This sounds fair to me.  While some benign URLs likely show up in console messages (e.g. chrome://gpu or chrome://serviceworker-internals), blocking them from loading on click is consistent with other places.


I prefer to not regress styled links, and address the issue by blocking restricted URLs the same way that renderer does. 
 What do others think?  If that sounds fine, I'll take a look.
After an offline discussion, I'm also fine with regressing styled links in favor of a secure experience viewing links in Console.
An alternative to regressing on styling is to add an intermediate confirmation view before navigating away when a styled link is detected. I'd use a whitelist for the logic that determines whether such a prompt is shown (e.g. allowing font sizes within certain ranges).
Project Member

Comment 8 by sheriffbot@chromium.org, Jan 3

Labels: M-64
Project Member

Comment 9 by sheriffbot@chromium.org, Jan 3

Labels: Pri-1
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 8

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

commit 3e0faa6d6c13dda5ec3f906a8d2951c3ad115645
Author: Erik Luo <luoe@chromium.org>
Date: Mon Jan 08 23:36:49 2018

Revert "DevTools: ensure console links are always visible"

This reverts commit 38391374b79c006e85e3b9c912267a8d4daaa7cc.

Reason for revert: This led to individual horizontal scrollbars on long messages.
Screenshot: https://imgur.com/a/Tiec9

Original change's description:
> DevTools: ensure console links are always visible
> 
> - The %c style formatter will not apply to links in console
> - Messages now have 'overflow: hidden'
> 
> Screenshot: https://imgur.com/a/7tAj9
> 
> Bug:  798096 
> Change-Id: Ifc31314cec2a286e58ebb5625348f8dfe2827d6e
> Reviewed-on: https://chromium-review.googlesource.com/849650
> Commit-Queue: Erik Luo <luoe@chromium.org>
> Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#527501}

TBR=dgozman@chromium.org,luoe@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  798096 
Change-Id: I54c74a9b6c3cf5b604df5659b01fa99302998e56
Reviewed-on: https://chromium-review.googlesource.com/855065
Reviewed-by: Erik Luo <luoe@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527812}
[modify] https://crrev.com/3e0faa6d6c13dda5ec3f906a8d2951c3ad115645/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-style-expected.txt
[modify] https://crrev.com/3e0faa6d6c13dda5ec3f906a8d2951c3ad115645/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-style.js
[modify] https://crrev.com/3e0faa6d6c13dda5ec3f906a8d2951c3ad115645/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
[modify] https://crrev.com/3e0faa6d6c13dda5ec3f906a8d2951c3ad115645/third_party/WebKit/Source/devtools/front_end/console/consoleView.css

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 11

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

commit 9495ba2aeac7df698fdcc6959a3534d34adc98c9
Author: Erik Luo <luoe@chromium.org>
Date: Thu Jan 11 02:44:45 2018

Reland "DevTools: ensure console links are always visible"

Instead of using 'overflow-y: hidden' to hide overflow,
CSS containment rules can make sure that the element's
paint will not affect other elements.

Screenshot: https://imgur.com/a/jAWI7

This is a reland of 38391374b79c006e85e3b9c912267a8d4daaa7cc
Original change's description:
> DevTools: ensure console links are always visible
>
> - The %c style formatter will not apply to links in console
> - Messages now have 'overflow: hidden'
>
> Screenshot: https://imgur.com/a/7tAj9
>
> Bug:  798096 
> Change-Id: Ifc31314cec2a286e58ebb5625348f8dfe2827d6e
> Reviewed-on: https://chromium-review.googlesource.com/849650
> Commit-Queue: Erik Luo <luoe@chromium.org>
> Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#527501}

Bug:  798096 
Change-Id: I43f151a4c91e6f084c91017753c74e9cb4a7e08c
Reviewed-on: https://chromium-review.googlesource.com/854957
Commit-Queue: Erik Luo <luoe@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528539}
[modify] https://crrev.com/9495ba2aeac7df698fdcc6959a3534d34adc98c9/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-style-expected.txt
[modify] https://crrev.com/9495ba2aeac7df698fdcc6959a3534d34adc98c9/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-style-whitelist-expected.txt
[modify] https://crrev.com/9495ba2aeac7df698fdcc6959a3534d34adc98c9/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-style.js
[modify] https://crrev.com/9495ba2aeac7df698fdcc6959a3534d34adc98c9/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js

Project Member

Comment 13 by sheriffbot@chromium.org, Jan 17

luoe: 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
Status: Started (was: Assigned)
Update: the commit landed in comment #12 forces styled links to be visible, which mitigates the original vulnerability, since it should no longer be possible to hide a link with the style formatter.

I still plan to land another patch to limit the allowed URLs, just to be more consistent with the renderer's filter.
Project Member

Comment 15 by bugdroid1@chromium.org, Jan 24

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

commit 3fe960d294e147d487e241ccbe469929d1fdb660
Author: Erik Luo <luoe@chromium.org>
Date: Wed Jan 24 21:39:13 2018

DevTools: prevent OpenInNewTab from opening browser debug URLs

Clicking in DevTools can open more links than renderer can.
This CL restricts DevTools from opening external URLs that are
for browser-debugging, such as 'chrome://crash'.

Bug:  798096 
Change-Id: Idc969ddf6da1a24c6aca172565fe80485fa4cc9d
Reviewed-on: https://chromium-review.googlesource.com/862384
Reviewed-by: Ɓukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531689}
[modify] https://crrev.com/3fe960d294e147d487e241ccbe469929d1fdb660/chrome/browser/devtools/devtools_sanity_browsertest.cc
[modify] https://crrev.com/3fe960d294e147d487e241ccbe469929d1fdb660/chrome/browser/devtools/devtools_window.cc

Status: Fixed (was: Started)
Sorry, I noticed after this was merged that I failed to update the description.  For reference, this CL in comment #15 not only restricts debug urls, but also filters out other urls that normal renderers do.
Labels: -Security_Severity-Medium Security_Severity-Low
We discussed this and a similar bug ( bug 798096 ) offline:
- Without any mitigating factors, navigations to chrome://* URLs (in particular to chrome://restart) could warrant severity-medium, as these could be chained with other bugs. We'll try to apply this guideline consistently to future bugs.
- However, in this report the user has to open devtools and click on a link, which we consider a significant mitigating factor.
 
rob@robwu.nl: Given that, I'm afraid I'm going to change this to low severity as well. Thank you again for the detailed report and the nice PoC.
^ That should say "a similar bug (  bug 797525  )" instead.
> - However, in this report the user has to open devtools and click on a link, which we consider a significant mitigating factor.

The user only needs to click *anywhere* in the console once the DevTools is opened.
Thus, the two vulnerabilities in this bug can result in very practical attack scenarios on Web Developers (comment #4).

Is the "behave like a web developer" requirement a sufficient mitigation to drop a security bug to low?


PS. Should M-64 be changed to M-65?
Yes, any non trivial user interaction is a sufficient mitigation per https://chromium.googlesource.com/chromium/src/+/master/docs/security/severity-guidelines.md
Labels: reward-topanel
Project Member

Comment 22 by sheriffbot@chromium.org, Feb 8

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -reward-topanel reward-0
I'm afraid the VRP panel declined to reward for this report.
Labels: -M-64 M-66
Labels: Release-0-M66
Labels: CVE-2018-6112
Labels: CVE_description-missing
Project Member

Comment 28 by sheriffbot@chromium.org, May 3

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