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 2 users
Status: Fixed
out until 25 sept
Closed: Oct 2016
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Sign in to add a comment
Security:Chrome Address Bar URL Spoofing
Reported by, Oct 20 2016 Back to list
Chrome Address Bar URL Spoofing

Chrome Version: 54.0.2840.59 (64-bit)+[Stable]
Operating System: [MAC OS 10.12, Windows 7&10]

Right click on the link, select open a page in a new window. Then,in the address bar will be displayed the first request URL as the final committed URL .


<a href="">11111</a>
<a href="">22222</a>
<a href="">33333</a>

Online demo:

471 bytes View Download
Components: UI>Browser>Omnibox
Status: Untriaged
Confirmed repro in latest Canary (56.0.2896.0). It looks like we fail to update the URL Bar with the final address after the browser gets back the redirect.
32.5 KB View Download
In debug, clicking in the Omnibox crashes a DCHECK:

[9184:7300:1020/] Check failed: false. Trying to get the spec of an invalid URL!
In older Chrome, navigation to the invalid URL failed with a blank white page. In current Chrome, navigation succeeds and the invalid URL is shown in the omnibox and fails to refresh on navigation.

You are probably looking for a change made after 316900 (known good), but no later than 316926 (first known bad).
The script might not always return single CL as suspect as some perf builds might get missing due to failure.

Status: Assigned
creis@, can you PTAL?

CL changed NavigationController::CreateNavigationEntry such that it calls a fixup function on the URL.

That fixup function changes a GURL with is_valid==true to one where is_valid==false.

e.g. annotating thusly:
std::unique_ptr<NavigationEntry> NavigationController::CreateNavigationEntry(
    const GURL& url,
    const Referrer& referrer,
    ui::PageTransition transition,
    bool is_renderer_initiated,
    const std::string& extra_headers,
    BrowserContext* browser_context) {
  // Fix up the given URL before letting it be rewritten, so that any minor
  // cleanup (e.g., removing leading dots) will not lead to a virtual URL.
  GURL dest_url(url);
  if (!url.is_valid())  {
    MessageBox(0, base::UTF8ToUTF16(url.possibly_invalid_spec()).c_str(), L"Incoming URL Invalid", 0);
  if (!dest_url.is_valid())
    MessageBox(0, base::UTF8ToUTF16(dest_url.possibly_invalid_spec()).c_str(), L"Fixed up URL Invalid", 0);

... We see only one messagebox:

Fixed up URL Invalid
Components: UI>Browser>Navigation
Summary of the Repro-- A GURL with spec


... gets passed into CreateNavigationEntry. This URL is of scheme "" (which is a legal scheme name, even though nobody has this protocol installed).

The FixupURLBeforeRewrite call added in Chrome 42.0.2309.0 "fixes up" this URL into: 

    "{eviltextomitted}" the scheme of the URL is now "HTTP" and there's only one colon after the hostname.

Then, the existing RewriteURLIfNecessary call in this function fixes up the URL again, such that it is now: 


... with no remaining colons after the hostname. This is the URL used to retrieve data from the network, while the Virtual URL stored on the Navigation Entry is the "" string.
Comment 7 by, Oct 21 2016
The problem is indeed in NavigationController::CreateNavigationEntry and the usage of the URL fixups. 

FixupURLBeforeRewrite ends up creating the broken URL because the parser considers valid host

(gdb) p domain
$33 = ""
(gdb) ...
581         if (chrome_url && !
(gdb) p

It then proceeds to construct this very weird looking URL. GURL considers it invalid, but CreateNavigationEntry never checks that. 

However, the RewriteURLIfNecessary after that rewrites it to be a valid URL without setting the reverse_on_redirect boolean to true. This one is use to undo the rewrite if a redirect is encountered, so we just leave the URL intact on the commit of the redirected navigation.

I missed to check which handler cleaned up the URL during RewriteURLIfNecessary, but will continue investigating on Monday.
I've confirmed that this problem predates the M42 change cited in Comment #4; that change simply increased the number of colons required from one to two (e.g. in M39, "" was the exploit string, in M42 it became "").

I also found that the exploit doesn't require a clientside redirectpr (e.g. META Refresh page used in the original repro). A victim server with an open HTTP/302 redirector is vulnerable too.
Labels: Security_Severity-Medium Security_Impact-Stable OS-All
Project Member Comment 10 by, Oct 23 2016
Labels: M-55
Project Member Comment 11 by, Oct 23 2016
Labels: Pri-1
Project Member Comment 12 by, Oct 24 2016
Labels: M-55
Comment 13 by, Oct 24 2016
Status: Started
Nasko is making progress, so I'll assign to him.  (We'll chat about potential fixes today.)
Comment 14 by, Oct 24 2016
The URL fixing (FixupURLBeforeRewrite) is done by functionality external to content/, which means that any content/ user can run into issues if it returns invalid URL from its implementation.

After a discussion with creis@ the current plan is to update CreateNavigationEntry to fail the creation for renderer initiated navigations. 
For browser-initiated ones, Chrome doesn't navigate to the invalid URL, but rather sends it to the user's default search engine. The remaining issue is renderer initiated navigations, for which the pending navigation entry is more of convenience/nice usability than actual functionality. Therefore it should be safe to implement this and backport it to previous milestones.

I'll put together a CL with this approach today/tomorrow. elawrence@, if you are interested in helping out, let me know and we can collaborate.
Comment 15 by, Oct 25 2016
It actually doesn't reproduce with renderer-initiated navigations. It is only a problem with using "Open in a new tab" from the context menu, which does a browser initiated navigation with the raw URL. When using a Ctrl+click to open in a new tab it uses RFH::OpenURL and the URL is cleaned up to be properly formatted somewhere in the browser process between RFH::OpenURL and RFH::Navigate. I will track that down to understand it.
Comment 16 by, Oct 26 2016
Another update on my investigation. There are multiple variations of navigating to the malformed URL (containing ::).

* Click on a regular link, click on targeted link, - this is a renderer-initiated navigation to the malformed URL. It is considered valid with a scheme of and it is passed to the external procol handler, which pops up a dialog to the user. There is no spoof in these cases.

* Ctrl+click, Shift+click, middle click - this is a renderer-initiated navigation with disposition (new tab/window) and is handled through the OpenURL IPC. The browser handles it, creates the new tab/window and instructs the renderer to navigate. This creates a pending NavigationEntry with different URL and virtual URL, which is what usually would cause the URL spoof. At this point, the URL used for loading is the cleaned up "" URL and the virtual URL is the one containing the colon - "". When the renderer process starts the navigation, it sends a DidStartProvisionalLoad IPC to the browser. NavigatorImpl::DidStartMainFrameNavigation checks whether there is a pending NavigationEntry for browser-initiated navigation. There is one, but it is for renderer-initiated one, so it just creates a new pending NavigationEntry. That one is created with the URL we are navigating to, which is the cleaned up URL, which doesn't get further rewritten and there is no virtual URL set on that NavigationEntry. From that point on there is no spoof, since there is no virtual URL to become stale.

* Right-click, "Open a link in a new *" - this is a browser-initiated navigation with the malformed URL. It creates a tab and instructs it to navigate. The pending NavigationEntry is created with the cleaned up URL of "" and the virtual URL is set to the fixed up URL "". The call to RewriteURLIfNecessary does not set the reverse_on_redirect boolean to true, so from that point on the virtual URL is not touched on redirects and becomes stale, leading to the URL spoof.

* Navigating from the omnibox - in this case, the Omnibox code takes the string and the AutocompleteClassifier::Classify method returns an URL for performing a search - "". There is no issues with spoofing here, as the tab navigates to the search page for the string supplied in the omnibox.
Comment 17 by, Oct 26 2016
Adding boliu@ for context, since I pulled him in on the code review.
Comment 18 by, Oct 26 2016
I have a CL out for review and should land within a day.
Project Member Comment 19 by, Oct 27 2016
The following revision refers to this bug:

commit e4ebe078840e65d673722e94f8251b334030b5e8
Author: nasko <>
Date: Thu Oct 27 16:52:17 2016

Drop navigations to NavigationEntry with invalid virtual URLs.

BUG= 657720 

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


Comment 20 by, Oct 27 2016
Status: Fixed
This should be fixed in the next canary release.
Labels: reward-topanel
Project Member Comment 22 by, Oct 28 2016
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Comment 23 by, Oct 28 2016
Labels: Merge-Request-55
The fix has been on canary for mostly a day and no obvious crashes. Requesting a merge to M55.
Comment 24 by, Oct 28 2016
Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member Comment 25 by, Oct 29 2016
Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:

commit a6d3567d17b1922f6fb458fcd73ef4130d28cd51
Author: Nasko Oskov <>
Date: Fri Oct 28 23:50:38 2016

Drop navigations to NavigationEntry with invalid virtual URLs.

BUG= 657720 

Cr-Commit-Position: refs/heads/master@{#428056}
(cherry picked from commit e4ebe078840e65d673722e94f8251b334030b5e8)

Review URL: .

Cr-Commit-Position: refs/branch-heads/2883@{#373}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}


Adding awhalley@ to weigh in whether this needs to be merged in M54.
Labels: -reward-topanel reward-unpaid reward-500
Congratulations, the panel awarded $500 for this report!
Comment 29 by, Nov 7 2016
Thanks! Please use 'xisigr of Tencent's Xuanwu Lab' as the credit information when you're ready to release a newer version of Chrome.
Labels: -reward-unpaid reward-inprocess
Labels: -Hotlist-Merge-Approved
Labels: Release-0-M55
Labels: CVE-2016-5222
Project Member Comment 34 by, Feb 3 2017
Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit - Your friendly Sheriffbot
Sign in to add a comment