New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 77507 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2011
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

URL Bar Spoof

Reported by kuz...@gmail.com, Mar 26 2011

Issue description

Test chrome 12.0.712.0 dev windows xp sp3

1,Open testcase.htm
2,Click 'clickme'


 
testcase.htm
408 bytes View Download
Cc: darin@chromium.org a deleted user
Labels: -Pri-0 -Area-Undefined Pri-1 Area-WebKit OS-All SecSeverity-High Mstone-11
Status: Available
hits this assert 

/*
 There is a race condition between the layout and load completion that affects restoring the scroll position.
 We try to restore the scroll position at both the first layout and upon load completion.
 
 1) If first layout happens before the load completes, we want to restore the scroll position then so that the
 first time we draw the page is already scrolled to the right place, instead of starting at the top and later
 jumping down.  It is possible that the old scroll position is past the part of the doc laid out so far, in
 which case the restore silent fails and we will fix it in when we try to restore on doc completion.
 2) If the layout happens after the load completes, the attempt to restore at load completion time silently
 fails.  We then successfully restore it when the layout happens.
*/
void HistoryController::restoreScrollPositionAndViewState()
{
    if (!m_frame->loader()->stateMachine()->committedFirstRealDocumentLoad())
        return;

    ASSERT(m_currentItem);

Charlie, it would be great if you add this to your bunch of url spoof bugs.

Comment 2 by creis@chromium.org, Mar 31 2011

Cc: -a deleted user a deleted user security...@gtempaccount.com
Owner: a deleted user
Status: Assigned
Sure, I'll take a look early next week when I get back in town.  CC'ing Mihai just as a heads up, since that's in HistoryController.

Comment 3 by creis@chromium.org, Apr 6 2011

Status: Started
There's a strange WebKit bug at the heart of this.  Turns out the following code does the wrong thing:
w = window.open(any_url);
w.document.write(1);
w.location.reload();

Instead of reloading any_url, we put the contents of the *opener* window in w.  I've filed http://webkit.org/b/57906 to track it.

The HistoryController crash in comment 1 is somewhat of a red herring.  It's already being discussed at http://webkit.org/b/50331.

Comment 4 by creis@chromium.org, Apr 6 2011

Cc: brettw%c...@gtempaccount.com abarth%c...@gtempaccount.com
Hmm, it turns out the behavior described in comment 3 is intentional.  Adam points out that the document.write call causes the opener window to take over the new window, giving the new window its URL and security context.

That means we need to fix the aftermath of this situation.

1) NavigationController is currently ignoring the FrameNavigate message for the reload because it arrives with a page ID of -1.  Either we need to give it a valid page ID in the renderer or we need to stop ignoring it in NavigationController.  I'm trying to understand which is the right approach.

2) We may want to consider also sending a FrameNavigate message from the renderer after a document.write call.  This is how Firefox behaves-- it updates the URL bar immediately after the document.write call to show the URL of the calling page.

Comment 5 Deleted

Comment 6 by creis@chromium.org, Apr 8 2011

Re: comment 5, yes, both test cases are due to the same bug.  Both set up a cross-process navigation and interrupt it with a navigation from the original renderer process.  (The test case attached here uses window.open() and w.opener=null, and the test case from 78384 uses view-source:.  Both result in the navigation happening in a separate renderer process.)

The tricky part about this bug is that the reload happens on a "page" without a NavigationEntry-- essentially on the about:blank page that we typically discard after the first successful navigation in the tab.  As a result, the FrameNavigate message has a page_id of -1, which causes NavigationController to ignore it.  I don't think we want to give it a page_id, because we intentionally don't keep this "page" in the browser history if you navigate away.

I'm working on a CL that tries to cancel the pending entry and refresh the URL bar if something like this happens, but it may need some more work.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 12 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=81307

------------------------------------------------------------------------
r81307 | creis@chromium.org | Tue Apr 12 14:17:49 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/tab_contents/navigation_controller.cc?r1=81307&r2=81306&pathrev=81307
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/tab_contents/navigation_controller_unittest.cc?r1=81307&r2=81306&pathrev=81307

Ensure URL is updated after a cross-site navigation is pre-empted by
an "ignored" navigation.

BUG= 77507 
TEST=NavigationControllerTest.LoadURL_IgnorePreemptsPending

Review URL: http://codereview.chromium.org/6826015
------------------------------------------------------------------------

Comment 8 by creis@chromium.org, Apr 12 2011

Status: Fixed
Fixed in r81307.
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify ReleaseBlock-Stable reward-topanel
Status: WillMerge
Thanks Charlie! We'll merge to M11.
Status: FixUnreleased
Merged to M11 at r81791
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 15 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=81791

------------------------------------------------------------------------
r81791 | cevans@chromium.org | Fri Apr 15 13:16:15 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/696/src/content/browser/tab_contents/navigation_controller_unittest.cc?r1=81791&r2=81790&pathrev=81791
 M http://src.chromium.org/viewvc/chrome/branches/696/src/content/browser/tab_contents/navigation_controller.cc?r1=81791&r2=81790&pathrev=81791

Merge 81307 - Ensure URL is updated after a cross-site navigation is pre-empted by
an "ignored" navigation.

BUG= 77507 
TEST=NavigationControllerTest.LoadURL_IgnorePreemptsPending

Review URL: http://codereview.chromium.org/6826015

TBR=creis@chromium.org
Review URL: http://codereview.chromium.org/6865026
------------------------------------------------------------------------
Labels: -reward-topanel reward-1000 reward-unpaid
@kuzzcc: another URL bar spoof, and to our surprise, a different root cause (and code change)! Also, a nice simple test case. Therefore we're rewarding this provisionally at the $1000 level for a Chromium Security Reward -- nice one!

----
Boilerplate text:
Please do NOT publicly disclose details until a fix has been released to all our
users. Early public disclosure may cancel the provisional reward.
Also, please be considerate about disclosure when the bug affects a core library
that may be used by other products.
Please do NOT share this information with third parties who are not directly
involved in fixing the bug. Doing so may cancel the provisional reward.
Please be honest if you have already disclosed anything publicly or to third parties.
----
Labels: CVE-2011-1446
Labels: -reward-unpaid
Invoice finalized; payment is in e-payment system; it can take a couple of weeks.
Labels: SecImpacts-Stable
Batch update.
Lifting view restrictions.
Labels: -Restrict-View-SecurityNotify
Lifting view restrictions.
Status: Fixed
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Type-Security -Area-WebKit -SecSeverity-High -Mstone-11 -SecImpacts-Stable Cr-Content Security-Impact-Stable Security-Severity-High M-11 Type-Bug-Security
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Severity-High Security_Severity-High
Project Member

Comment 23 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member

Comment 24 by bugdroid1@chromium.org, Apr 6 2013

Labels: -Cr-Content Cr-Blink
Project Member

Comment 25 by sheriffbot@chromium.org, Oct 1 2016

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
Project Member

Comment 26 by sheriffbot@chromium.org, Oct 2 2016

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
Labels: allpublic
Labels: CVE_description-submitted

Sign in to add a comment