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.
Issue 280512 Possible to hide current address by going to "tel:" link and then a "#" link
Starred by 1 user Reported by, Aug 28 2013 Back to list
Status: Fixed
Closed: Sep 2013
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Sign in to add a comment
UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.14 Safari/537.36

Steps to reproduce the problem:
1. Click on a link starting with "tel:"
2. Click on a link starting with "#"

What is the expected behavior?
To now be at [current page]#something

What went wrong?
The URI has disappeared and it only shows the tel: address

Did this work before? N/A 

Chrome version: 30.0.1599.14  Channel: beta
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: Shockwave Flash 11.8 r800

The attached image shows what I see when I open the attached .html file (works both when opening from the file system as well as opening from an http server)
200 bytes View Download
4.2 KB View Download
Comment 1 by, Aug 28 2013
Status: Untriaged
The scheme is clearly wrong, so I don't think this is viable as a spoof. However, it is really weird. Maybe one of the CCs has a better sense of what's going on here?
Comment 2 by, Aug 28 2013
Status: Assigned
Sounds similar to  issue 266922 , but it still repros after that was fixed.  Probably a pending entry issue.  I'll take a look.
Comment 3 by, Aug 29 2013
Labels: -Pri-2 -OS-Windows Pri-1 OS-All Cr-UI-Browser-Navigation M-29
Yep, it's essentially the same thing as  issue 266922 , except that fragment links like <a href="#foo"> don't go through DidStartProvisionalLoad, so my earlier fix doesn't work in this case.  I'll look into a fix.
Labels: Security_Impact-Stable Security_Impact-Beta
Fix labels.
Labels: Security_Severity-High reward-topanel
Comment 6 by, Sep 4 2013
Status: Started
I'm looking into this one, but it's tricky because we intentionally do not clear the pending entry in DidFailProvisionalLoad for some cases.  Still trying to find a safe way to fix it.
Comment 7 by, Sep 4 2013
Ok, assuming that we don't get a DidStartProvisionalLoad for all navigations (e.g., in-page navigations like fragments and pushState), then two of the possible ways to fix it are:

1) Clear the pending entry if we get to DidFailProvisionalLoad.
We don't seem to want to do this in general, since it means clicking stop on a slow URL can make the URL you typed or clicked disappear if you switch tabs and come back.  That was one of the points of fixing  issue 9682 .

2) Don't copy info like the virtual URL from the pending entry into the committed entry when we commit.
We do want to keep the virtual URL behavior for things like error pages and view source, though.

I'd love to revisit the lifetimes of pending entries (since keeping them around after they fail seems unfortunate), but I think there's a way to fix this for now in a more specific way we can merge to the branches.  Specifically, if the pending entry is not visible to the user (according to GetVisibleEntry), then there's no reason to keep it around in DidFailProvisionalLoad.

That covers attacks where a renderer-initiated navigation fails in a tab that has existing navigation entries (e.g., these repro steps).  A renderer-initiated navigation in a new tab would continue to be visible, but then a fragment or other in-page navigation would cause a DidStartProvisionalLoad on the initial empty document.  I think that means we both prevent the attack and don't cause failed URLs to disappear when the user might want to retry them.

I'll put together a CL with this approach.  It may help with issue 278899 as well.
Comment 9 by, Sep 10 2013
Status: Fixed
Just verified this is fixed in today's 31.0.1626.0 canary.
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved
Please merge your change to the m30 branch (1599) by early next week [using drover]. We have m30 beta coming next week and we want all the security changes in by that time. 
Comment 12 by, Sep 12 2013
Comment 11: I asked Karen about this, and she asked me to wait until Friday to merge it.  I'll take care of it on Friday.
Labels: -M-29 M-30 Merge-Merged Release-0
Did you saw our new criteria for possibly issuing higher rewards? See
E.g. If you are able to provide a repro that faulted at an address of 0x41414141, it will qualify for the new higher rewards. Or, if you can show that you have control between free and crash points, etc.
Labels: -Security_Severity-High Security_Severity-Medium
Sounds like there are mitigating factors that take this down to a medium?
Cc:, what name would you like us to use when we give you credit for this bug in the release notes on the Chrome blog?
"Wander Groeneveld" would be fine, thanks for fixing it
Labels: CVE-2013-2915
@mbarbella: sure, feel free to call it a Low
Comment 21 by, Sep 27 2013
Labels: -Security_Severity-Medium Security_Severity-Low
Labels: -reward-topanel
Not a realistic spoof.
Project Member Comment 23 by, Feb 6 2014
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 24 by, Feb 2 2016
Labels: -Security_Impact-Beta
Project Member Comment 25 by, Oct 1 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit - Your friendly Sheriffbot
Project Member Comment 26 by, Oct 2 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

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