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 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2011
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security
M-9

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
link

Issue 54262: Possible Location Bar & SSL Spoofing

Reported by jconsult...@gmail.com, Sep 2 2010

Issue description

Chrome Version       : 6.0.472.53

Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari 4:FAIL
  Firefox 3.x:FAIL
         IE 7:FAIL
         IE 8:FAIL

What steps will reproduce the problem?
1. Open a new tab and go to https://www.alternativ-testing.fr/chromeSpoof56f4d654fd/index5.php
2. go to https://www.alternativ-testing.fr/chromeSpoof56f4d654fd/index5.php#test
3. Reload and Back

What is the expected result?
The location bar is Spoofed with valid SSL/TLS certificate.
 
spoof.png
107 KB View Download
Showing comments 40 - 139 of 139 Older

Comment 40 by jconsult...@gmail.com, Oct 27 2010

I would like know if this issue is valid for a reward?
And can i write a blog post about this ?

Comment 41 by scarybea...@gmail.com, Oct 28 2010

@jordi: I'll send it over to the rewards panel once we have found the right person to work on it and have a code fix. We usually like to do things that way because the panel can then convene with a full understanding of what exactly was wrong in the code.
I don't have an ETA yet, sorry.

In terms of the blog post -- we do encourage people to share their discoveries with the world. However, we ask that people hold off on doing this until we have a patch out to our users. Premature blogging may not be looked on favourably by the rewards panel.

Comment 42 by kerz@chromium.org, Oct 28 2010

Is this needed for m8?

Comment 43 by infe...@chromium.org, Oct 28 2010

Jason, yes, we need an owner here. Brett, can you please help.

Comment 44 by abarth@chromium.org, Oct 30 2010

Owner hot potato.

Comment 45 by abarth@chromium.org, Oct 30 2010

Let me see if I understand the issue:

1) Tab A creates a new tab B.
2) Tab B navigates itself to a fragment and then reloads itself.
3) The reload triggers a redirect to the victim site.
4) Tab A then navigates tab B back one history item.

Comment 46 by abarth@chromium.org, Oct 30 2010

From that sequence, it sounds like the navigation entry created by the fragment navigation is incorrectly modified when processing the reload.

I suspect the confusion is related to the problem that the reload isn't supposed to create a new navigation entry, but with the redirect (which also doesn't create a new navigation entry) takes us to a new location.

Comment 48 Deleted

Comment 49 by jconsult...@gmail.com, Oct 30 2010

1) Tab A creates a new tab B.
2) Tab B navigates itself to a fragment and then reloads itself.
3) The reload triggers a redirect to the victim site.
4) Tab A then navigates tab B back one history item.
5) Tab A use location.href on tab B.
6) Tab A use history.forward on tab B.

Comment 50 by abarth@chromium.org, Oct 30 2010

I have a local repro.  It's seems like the thing that's confusing the navigation controller is the redirect during the reload.

Comment 51 by abarth@chromium.org, Oct 30 2010

The fragment navigation appears essential also.

Comment 52 by abarth@chromium.org, Oct 30 2010

Looking the debugger (for my reduced test case) the navigation controller seems to have the right understanding about what's going on.  It looks like the problem might be in the rendering engine.

/me => sleep.  Will look more tomorrow.

Comment 53 by abarth@chromium.org, Oct 30 2010

The way Safari solves this problem is interesting.  They show the same web page as we do, but they don't update the location bar the way we do.  That might be the least-injury way of fixing this bug.

Comment 54 by abarth@chromium.org, Oct 31 2010

I think the issue is that the document sequence number is being re-used during the reload even though we're actually creating a new document.

Comment 55 by abarth@chromium.org, Oct 31 2010

I think this patch fixes the issue.  I need to test it to make sure it doesn't break other things though.

Index: WebCore/history/HistoryItem.cpp
===================================================================
--- WebCore/history/HistoryItem.cpp	(revision 70977)
+++ WebCore/history/HistoryItem.cpp	(working copy)
@@ -237,6 +237,8 @@
 void HistoryItem::setURL(const KURL& url)
 {
     pageCache()->remove(this);
+    if (!equalIgnoringFragmentIdentifier(this->url(), url))
+        m_documentSequenceNumber = generateSequenceNumber();
     setURLString(url.string());
     clearDocumentState();
 }

Comment 56 by jsc...@chromium.org, Oct 31 2010

Awesome. Thanks Adam.

Comment 57 by abarth@chromium.org, Oct 31 2010

So, that patch passes all the existing tests.  I'm going to write a test and upload it for review.  I'm not overly confident in the patch because this is a complicated area, but we'll see what reviewers think.

Comment 58 by abarth@chromium.org, Nov 1 2010

I'm getting distracted by other things I need to work on.  Would it be possible for someone on the security team to take the ball from here?

Comment 59 by jsc...@chromium.org, Nov 1 2010

Sure, but what's left to do?

Comment 60 by abarth@chromium.org, Nov 1 2010

Mostly just writing a test.

Comment 61 by jsc...@chromium.org, Nov 1 2010

Gotcha. I'll grab it and take a crack at the layout test. Is there an upstream bug already or will I need to file one?

Comment 62 by abarth@chromium.org, Nov 1 2010

No upstream bug yet.

Comment 63 by infe...@chromium.org, Nov 2 2010

Comment 64 by jsc...@chromium.org, Nov 2 2010

@jnd - Has graciously volunteered to walk this fix the last mile.

Comment 65 by jconsult...@gmail.com, Nov 5 2010

I've found a way for steal login and password saved . But require complicated step with minimal user interaction .

Comment 66 by jconsult...@gmail.com, Nov 5 2010

1) go to https://www.linkedin.com/secure/login and save your login & password
2) Go to http://www.alternativ-testing.fr/Research/Google%20Chrome/Google%20Chrome%20SSL%20Spoofing/passwordsteal/ and click on the link
3) Try to connect with saved password.
4)Login & Password are sent to ALTERNATIV-TESTING.fr

Comment 67 by jnd@chromium.org, Nov 12 2010

I tried to write a layout test to reproduce this bug for the webkit patch, however my test couldn't get the spoof situation in my either chrome build(9.0.580.0 Developer Build 65872) or WebKit build.

@jconsultant.chancel, could you test a look at my test and point where I were wrong.
test-spoof.html
259 bytes View Download
spoofing.php
3.1 KB View Download

Comment 68 by infe...@chromium.org, Nov 12 2010

Johnny, can you please test against v8 beta (552) branch. It does reproduce there, if you can please make a testcase on that, then we should still check in the webkit fix from Adam alognwith your layouttest.

Comment 69 by jconsult...@gmail.com, Nov 13 2010

Try redirect('https://www.linkedin.com/secure/login', 302); because spoofing google.com don't work (dont't know why).

Comment 70 by jconsult...@gmail.com, Nov 13 2010

your testcase works on google chrome 8.0.552.200

Comment 71 by infe...@chromium.org, Nov 15 2010

+cc creis.

This might be fixed in https://bugs.webkit.org/show_bug.cgi?id=48809, http://trac.webkit.org/changeset/71437. Charlie, does it seem related ? This bug is not reproducing on trunk, but does reproduce on 552. So, it does look related to your history fix. We have a planned beta pretty soon, and if this is the one :), then we would probably want to merge this (or wait for the follow up patch).

Comment 72 by infe...@chromium.org, Nov 15 2010

Lets wait for Charlie's followup patch before merging the fix. See last comment in http://code.google.com/p/chromium/issues/detail?id=62156.

Comment 9 by project member creis@chromium.org, Nov 11 (3 days ago)

I think this is higher priority, and I'm actively working on it.  Right now, back/forward is broken in Chrome and test_shell for any fragment navigations (page.html#foo).

I have a draft of a fix at https://bugs.webkit.org/show_bug.cgi?id=48809, but I'm figuring out why it's interfering with popstate at the moment.

Comment 73 by jnd@chromium.org, Nov 15 2010

According to my analysis, When reloading the same page, due to some reasons, like cookie or server intended behavior, the returned contents might be different than the result we just got before reloading the same page.
In webkit, the historyItem created by fragment change (spoofing.php#123) shared same document documentSequenceNumber with previous one (spoofing.php), after the historyItem(spoofing.php#123) was reloaded, the url of that historyItem(spoofing.php#123) was replaced to new one (like linedin.com), so the documentSequenceNumber of historyItem(spoofing.php#123) was as same as the documentSequenceNumber of historyItem(linkedin.com), that is why the url bar was changed, but the contents did not.

Adam already had right analysis on #54, and his patch on #55 did fix this issue.

The attached files are a more simple test case (redirect to google.com) and resultant result.
Will file a webkit bug and provide corresponding patch with a layout test-case.
google.png
80.4 KB View Download
spoofing.php
3.0 KB View Download
test-spoof.html
195 bytes View Download

Comment 74 Deleted

Comment 75 by jconsult...@gmail.com, Nov 15 2010

Try redirect('https://www.linkedin.com/secure/login', 302); because google.com spoofing don't work

Comment 76 by jconsult...@gmail.com, Nov 15 2010

spoofchrome.png
38.5 KB View Download

Comment 77 by jnd@chromium.org, Nov 15 2010

@ jconsultant.chancel, google.com spoofing did work, you can try my test case. It's another type URL spoofing, the URL bar showed "xxx/spoofing.php", but the content was google.com. It can be reproduced on current stable version 7.0.517.44.

Comment 78 by creis@chromium.org, Nov 15 2010

@inferno: I do have a new WebKit patch under review (https://bugs.webkit.org/attachment.cgi?id=73802&action=review) for fixing  issue 62156 , as you point out.  It does sound like the patch could help with this bug, since it affects fragment navigations and content state updates.  Specifically, it's trying to solve a very related problem in  issue 58082 , where the content state of NavigationEntries can get swapped.

I haven't verified if it fixes this yet, but I can give it a try to find out.

Comment 79 by creis@chromium.org, Nov 15 2010

FYI, my CL doesn't fix this issue, so don't wait for it.  When visiting http://www.alternativ-testing.fr/Research/Google%20Chrome/Google%20Chrome%20SSL%20Spoofing/TestCase5fd4df654df/, I see the Verisign page with the alternativ-testing.fr URL in the Omnibox.

Adam's patch to HistoryItem does cause the correct URL (sealinfo.verisign.com) to display in the Omnibox, but it looks like things still break if you click Forward and then Back.  Forward adds a #123 fragment to the URL, and Back leads to the original problem-- alternativ-testing.fr showing in the Omnibox but with the Verisign page displayed.  (I have some other changes in my tree, though, so others should see if they can reproduce this.)

Comment 80 by pkasting@chromium.org, Nov 15 2010

Comment 81 by jnd@chromium.org, Nov 16 2010

@creis, the following is my analysis.

#123 fragment in the final URL is because of a logic in URLRequestJob::NotifyHeadersComplete(url_request_job.cc, line 462), it says it moves the reference fragment of the old location to the new one if the new one has none and it duplicates mozilla's behavior.

In the case http://www.alternativ-testing.fr/Research/Google%20Chrome/Google%20Chrome%20SSL%20Spoofing/TestCase5fd4df654df/, the new TabB was from index5.php, changed location to index5.php#123, then reloaded to sealinfoxxx#123, it created two historyItems, index5.php and from sealinfoxxx#123(from index5.php#123)

Af first, the TabA called TabB's history.back, the history change was from sealinfoxxx#123 to index5.php. According to the loadType(FrameLoadTypeIndexedBackForward), the cache policy was ReturnCacheDataElseLoad(FrameLoader::navigateToDifferentDocument), so the cached data for index5.php was used, which redirected from index5.php to sealinfoxxx. In here sealinfoxxx was not a historyItem, it's just a redirection result and navigation entry of historyItem:index5.php.

So when you forwarded, the history change was from index5.php to sealinfoxxx#123, since the current display URL was sealinfoxxx, so you saw the url location from sealinfoxxx to sealinfoxxx#123.

Yes, the next back led to the original problem, it's because another webkit bug. In this previous forward, since the only difference between current doc URL: sealinfoxxx(historyItem:index5.php) and current target URL:sealinfoxxx#123 was the fragment #123, it was consider as navigation in same doc, so historyItem:index5.php was set to same documentState of sealinfoxxx#123, but actually those two historyItems should not be same. then in back operation, changing sealinfoxxx#123 to index5.php was also considered as navigation in same doc, so you saw wrong URL in location URL. 

The solution to fix this issue is to only allow copying the documentState from a historyItem only the two historyItems have same URL with ignoring fragment identifier. Please refer to the following patch.
 PassRefPtr<HistoryItem> HistoryController::createItemTree(Frame* targetFrame, bool clipAtTarget)
 {
     ...
         if (m_previousItem) {
             if (m_frame != targetFrame)
                 bfItem->setItemSequenceNumber(m_previousItem->itemSequenceNumber());
+            if (equalIgnoringFragmentIdentifier(m_previousItem->url(), bfItem->url()))
                 bfItem->setDocumentSequenceNumber(m_previousItem->documentSequenceNumber());
         }
     ...
 }

Comment 82 by jnd@chromium.org, Nov 17 2010

created a associated webkit bug: https://bugs.webkit.org/show_bug.cgi?id=49654

Comment 83 by darin@chromium.org, Nov 17 2010

Comment 84 by jconsult...@gmail.com, Nov 27 2010

do you have an idea of the release date with this issue ?

Comment 85 by jsc...@chromium.org, Jan 8 2011

Darin grabbed this upstream, so reassigning. Darin, not to nag, but have you got an eta on this? we really want to land a fix before m9 is released?

Comment 86 Deleted

Comment 87 Deleted

Comment 88 Deleted

Comment 89 Deleted

Comment 90 Deleted

Comment 91 Deleted

Comment 92 Deleted

Comment 93 by scarybea...@gmail.com, Jan 9 2011

Oh, dear. It looks like Jordi's PC has malware and someone is abusing it to send spam and/or make Jordi look really bad. I'll send him a personal note.

Comment 94 by jconsult...@gmail.com, Jan 9 2011

password changed ... sorry it was a bad friend who joked ...

Comment 95 by darin@chromium.org, Jan 20 2011

Comment 96 by infe...@chromium.org, Jan 20 2011

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Status: WillMerge

Comment 97 by scarybea...@gmail.com, Jan 20 2011

Labels: reward-topanel
@jconsultant.chancel: thanks again for your patience. As you can see, this was a tricky issue to fix properly. We now have a candidate code change to get to users.

The current plan is to ship the fix either in the upcoming Chrome 9, or in a patch shortly after Chrome 9. Not much longer now :)

Comment 98 by jconsult...@gmail.com, Jan 21 2011

This vulnerability can be used for spoof SSL/TLS & URL .
It can be used again to steal saved password & login with just a minimal interaction like a click.

I can give you a PoC for steal the login & the password with a simple click.

Comment 99 by scarybea...@gmail.com, Jan 21 2011

Labels: -SecSeverity-Medium SecSeverity-High
@jconsultant.chancel: yes, it can :) I've updated the severity to SecSeverity-High to reflect this.
As a SecSeverity-High bug, the rewards panel will discuss it once the bug is closer to getting released to users. Thanks for your continued discretion; as you can see, we're now a lot closer to have this resolved. The hard part (complicated code change) has been resolved.

Comment 100 by jconsult...@gmail.com, Jan 24 2011

when the reward-panel will discuss about this bug?

Comment 101 by scarybea...@gmail.com, Jan 24 2011

@jconsultant.chancel: as per previous message, "the rewards panel will discuss it once the bug is closer to getting released to users". A couple of weeks perhaps?

Comment 102 by k...@google.com, Feb 1 2011

Labels: -Mstone-8 Mstone-9

Comment 103 by scarybea...@gmail.com, Feb 3 2011

Labels: -reward-topanel reward-1000 reward-unpaid
@jconsultant.chancel: the rewards panel discussed this bug. We're provisionally rewarding it at the $1000 level. Congratulations!
We're rewarding above the base amount because you were helpful in providing lots of different testcases, and in discussing the severity.
Please refrain from blogging about it until fixed -- we're definitely fixing it in one of the Chrome 9 patches, probably in a couple of weeks.

Comment 104 by chromium...@gmail.com, Feb 9 2011

Labels: -Area-UI Area-WebKit

Comment 105 by infe...@chromium.org, Feb 10 2011

don't need m10 merge since branched earlier.

m9 merged in r78236 (painfully, had to skip layouttests since there were conflicts and had a small issue in int64_t naming in HistoryItem.h"

Comment 106 by infe...@chromium.org, Feb 10 2011

Status: FixUnreleased

Comment 107 by jconsult...@gmail.com, Feb 21 2011

fixed in 10.0.648.82 beta

Comment 108 by scarybea...@gmail.com, Feb 21 2011

Thanks for checking, Jordi. If you can just wait a little longer for us to release it to Stable users via a patch to Chrome 9, that would be awesome. We've already done the merge, just doing some QA.

Comment 109 by jconsult...@gmail.com, Feb 21 2011

Have you an idea of the release date?

Comment 110 by jconsult...@gmail.com, Mar 1 2011

the release is done , where is my reward?

Comment 111 by scarybea...@gmail.com, Mar 1 2011

One step ahead of you Jordi, I started the process about an hour ago :) Might take a couple of weeks; I've no idea why bank wires in 2011 aren't instant, but there you have it...
Thanks! This bug was interesting.

Comment 112 by jconsult...@gmail.com, Mar 1 2011

And now , can i write a blog post about this vulnerability? please reply quickly.

Comment 113 by jconsult...@gmail.com, Mar 1 2011

?

Comment 114 by jconsult...@gmail.com, Mar 1 2011

Now the fix is released , i think i can write a small blog-post without PoC.
let me know if you want delete of my blog post (without PoC).

Comment 115 by scarybea...@gmail.com, Mar 1 2011

Of course. Go right ahead.

Comment 116 by jconsult...@gmail.com, Mar 1 2011

what is the CVE id?

Comment 117 by jsc...@chromium.org, Mar 1 2011

We aren't a CVE assigning authority, so we currently don't provide or track CVEs.

Comment 118 by jconsult...@gmail.com, Mar 2 2011

this spoofing is exploitable on safari.
Have you reported it and credited me?

Comment 119 by scarybea...@gmail.com, Mar 2 2011

Hey Jordi -- Apple are aware and they know who to credit :)

Comment 120 by scarybea...@gmail.com, Mar 4 2011

Labels: -reward-unpaid
Invoice finalized; payment is in e-payment system.

Comment 121 by jconsult...@gmail.com, Mar 4 2011

On my last reward , multiple mails was sent for the wire transfer
but i don't have received any mail for this one.

do I have to wait a few weeks to send my IBAN number?

Comment 122 by scarybea...@gmail.com, Mar 4 2011

If the last reward worked in the end, hopefully this one should work too, without the need to give us the IBAN again.

All of the finance systems seem to do things in "batches" so you might get a few days or a week latency here and there.

Comment 123 by jconsult...@gmail.com, Mar 10 2011

I did not receive my reward, 
I think that there was an error in the transfer.

please reply quickly !

Comment 124 Deleted

Comment 125 by scarybea...@gmail.com, Mar 10 2011

Jordi, please be patient. There is nothing I can do to make an international wire transfer go faster. I see no indication that anything has gone wrong at this stage.

Comment 126 by jsc...@chromium.org, Mar 21 2011

Labels: Type-Security

Comment 127 Deleted

Comment 128 by jsc...@chromium.org, Oct 5 2011

Labels: SecImpacts-Stable
Batch update.

Comment 129 by jsc...@chromium.org, Apr 18 2012

Labels: -Restrict-View-SecurityNotify
Lifting view restrictions.

Comment 130 by jsc...@chromium.org, Apr 18 2012

Status: Fixed

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

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

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

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

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

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

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

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

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

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

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

Project Member
Labels: -Cr-Content Cr-Blink

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

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

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

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

Comment 139 by mbarbe...@chromium.org, Oct 2 2016

Labels: allpublic
Showing comments 40 - 139 of 139 Older

Sign in to add a comment