1247968 - (CVE-2020-6808) Full URL spoof using javascript URI scheme

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[:Gijs (he/him)]

Attachment: FF-URL-SPOOFv2.html

+++ This bug was initially created as a clone of Bug #1228754 +++

(original test and steps from bug 1228754 and @qab, not from me)

1. Open attached file.
2. click link
3. click back button in the browser


Somehow, the URL stays in the URL bar. Interrogating the browser's currentURI shows the URL shown in the URL bar, but the document displayed is actually about:blank (and the documentURI and so on verify this, and document.documentElement.nodePrincipal is the original opener's principal (localhost if you host the file there, bugzilla if you opened it directly ).

(In reply to Boris Zbarsky [:bz] from bug 1228754 comment #33)
> Sounds like it, but it also sounds like something different from the
> original issue reported in this bug (the one I analyzed in comment 6). 
> Seems like we should get a new bug filed on the interaction involving
> session history, and probably ask Olli whether he can take a look.

--> ni Olli.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

I thought I had taken this.

Trying to get to this once I've done the current reviews.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

And I so thought I had assigned this to myself. At least I had added this to my todo list.

Comment 3

[Daniel Veditz [:dveditz]]

Adding the bounty flag as requested by the reporter.

Lowering the severity to sec-moderate reflecting that
 1) the user has to hit back on the error page (unusual suspicion-raising circumstances)
 2) you can only spoof sites that don't actually have certs

#2 in particular means you can't spoof Facebook, Google, "my bank", etc. Spoofing is only useful if it can be used to fake a valuable target site for phishing, etc. attacks.

Comment 4

[Abdulrahman Alqabandi]

(In reply to Daniel Veditz [:dveditz] from comment #3)
> Adding the bounty flag as requested by the reporter.
> 
> Lowering the severity to sec-moderate reflecting that
>  1) the user has to hit back on the error page (unusual suspicion-raising
> circumstances)
>  2) you can only spoof sites that don't actually have certs
> 
> #2 in particular means you can't spoof Facebook, Google, "my bank", etc.
> Spoofing is only useful if it can be used to fake a valuable target site for
> phishing, etc. attacks.

You can spoof pages that contain in invalid character. So, for example, you can spoof a page that looks like 'https://www.facebook.com`' (note the invalid character), so its not just websites with no certs. In my opinion most users would not notice the invalid character at the end. Also I assume there is a special character that is less noticeable than '`' which could be used.

Comment 5

[Abdulrahman Alqabandi]

Attachment: Different approach to the same bug, javascript acts buggy with hash symbol.

Hope this will help in fixing.

Comment 6

[Al Billings [:abillings - ex-MoCo]]

Any update on this security bug?

Comment 7

[Al Billings [:abillings - ex-MoCo]]

Any updates here? This bug is lingering quite a bit.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Samael, do you think you'd have time to help here? Just say no if you don't have, and I'll try to
find time.
I've been slacking horribly with this one.

Comment 9

[Samael Wang [:freesamael] (away for now)]

As the prerendering stuff are progressing so slowly, I don't think I can take this bug recently...

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

ok, no problem.

Comment 11

[Samael Wang [:freesamael] (away for now)]

(In reply to Olli Pettay [:smaug] (vacation Nov 19-26) from comment #10)
> ok, no problem.

Got time for this. I'll try to figure it out.

Comment 12

[Samael Wang [:freesamael] (away for now)]

One thing I'm noticing is that Chrome doens't put javascript: into session history. As an extremely simple test, I did:

location.href="javascript:'whatever'";

In chrome, the document does change, but URL bar, history.length and location.href remain the same. In firefox both URL bar and location.href changed to javascript:'whatever', and history length increases.

I don't see a clear reason to keep javascript: in session history, maybe we should change this behavior.

BTW I think that's also the reason I sometimes keep being redirected again when pressing back. Addons such as Wikiwand is using javascript: to redirect. I have impression that some websites also do.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Per spec, history.length should increase.  Otherwise you get user dataloss: the user can't go back to the page that used to be there!

Also per spec the URL bar and location.href should become the URL of the document triggering the javascript: load or so.  At some point we should really sit down and align this last bit with the spec...  Pretty sure we have existing bugs tracking that.

Comment 14

[Samael Wang [:freesamael] (away for now)]

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #13)
> Per spec, history.length should increase.  Otherwise you get user dataloss:
> the user can't go back to the page that used to be there!
> 
> Also per spec the URL bar and location.href should become the URL of the
> document triggering the javascript: load or so.  At some point we should
> really sit down and align this last bit with the spec...  Pretty sure we
> have existing bugs tracking that.

I believe you're referring bug 836567. I found the spec for document.URL stuff [1] and am still trying to better understand how spec describes session history behaviors.

It looks to me that the document.URL stuff is all we need for this bug. I'll see what I can do.

[1] https://html.spec.whatwg.org/multipage/browsers.html#javascript-protocol

Comment 15

[Samael Wang [:freesamael] (away for now)]

Attachment: WIP

Hi Boris,

May I have your feedback and advices here? I'm trying to use the override URI as the URI for document, session history entry, and docshell's current URI, but I'm not quite confident about how it should be implemented. Does this look the right path to you?

Comment 16

[Boris Zbarsky [:bzbarsky]]

If we want to take the "change the URI" approach, why wouldn't we just have the javascript: channel return the URI we want from GetURI, and presumably set the LOAD_REPLACE flag so people ignore the originalURI?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Or perhaps even more simply, change which URI we pass to the NS_NewInputStreamChannel call in nsJSChannel::Init, if we know which URI we'll really want at that point.

Comment 18

[Boris Zbarsky [:bzbarsky]]

In fact, I expect that the GetURI the nsJSChannel returns doesn't matter; what matters is the stream channel's URI.

Comment 19

[Samael Wang [:freesamael] (away for now)]

Thanks Boris!

I will commit patches on bug 836567 since it's more reasonable to deal with the document.URL issue there, and revisit this bug afterward. I'm expecting I'll be able to just invalid this bug then.

Comment 20

[Daniel Veditz [:dveditz]]

(In reply to Samael Wang [:freesamael] from comment #19)
> I will commit patches on bug 836567

Any timeline on that? Doesn't seem to be much activity over there.

> and revisit this bug afterward. I'm expecting I'll be able to just invalid this bug then.

No, don't do that! Fixing a bug elsewhere doesn't make a related bug INVALID. You could maybe use "worksforme" for an old bug that magically goes away, but if it's a security bug and you know what fixed it (as in this case, bug 836567 presumably) then you need to mark it FIXED.

Comment 21

[Abdulrahman Alqabandi]

Looks like the 'String.contains' function doesn't exist anymore. Make sure to replace 

>"location.toString().contains('http')" 

into 

>"location.toString().startsWith('http')"

At line 13 of the original PoC attached in Comment 0 for the PoC work again.

Comment 22

[Samael Wang [:freesamael] (away for now)]

(In reply to Daniel Veditz [:dveditz] from comment #20)
> (In reply to Samael Wang [:freesamael] from comment #19)
> > I will commit patches on bug 836567
> 
> Any timeline on that? Doesn't seem to be much activity over there.
> 
> > and revisit this bug afterward. I'm expecting I'll be able to just invalid this bug then.
> 
> No, don't do that! Fixing a bug elsewhere doesn't make a related bug
> INVALID. You could maybe use "worksforme" for an old bug that magically goes
> away, but if it's a security bug and you know what fixed it (as in this
> case, bug 836567 presumably) then you need to mark it FIXED.

Sorry I wasn't actively working on this recently. Made some attempts these 2 days but according to bug 836567 comment 19 I should be holding it for a moment. And thanks for the suggestion on the bugzilla process which I admit I'm not quite familiar.

Comment 23

[Al Billings [:abillings - ex-MoCo]]

Any update here? This bug is getting very old.

Comment 24

[Samael Wang [:freesamael] (away for now)]

I'll try to get back to this next week. Keep the ni? for reminding.

Comment 25

[Samael Wang [:freesamael] (away for now)]

Updates will be on bug 836567

Comment 26

[Samael Wang [:freesamael] (away for now)]

Well, we need someone else to take on this for obvious reasons.

Comment 27

[Al Billings [:abillings - ex-MoCo]]

Since Boris took over bug 836567, I am ni? him here.

Comment 28

[Boris Zbarsky [:bzbarsky]]

OK, bug 836567 is fixed now. Is there still an issue here? The original testcase in this bug doesn't even give me a "go back" option (if it's the first thing I load in the tab). If it's not, I can go back just fine and the URL updates correctly.

Comment 29

[:Gijs (he/him)]

Yeah, this seems to be OK on nightly now.

Comment 30

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 75.0a1 (2020-02-12) and on Firefox 74.0b1 on Windows 10 x64, Ubuntu 16.04 and Mac OS X 10.14.

Comment 31

[Tom Ritter [:tjr]]

Attachment: advisory.txt

I cribbed sheppy's comment from https://bugzilla.mozilla.org/show_bug.cgi?id=836567#c68 for this one...