1718571 - (CVE-2021-38509) Javascript dialog remains active in window after redirect

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Ademar Nowasky Junior]

Attachment: index.html

Javascript dialog that is shown while a basic authentication dialog is displayed remains active in the window even after redirects.

When opening a new URL through window.open(), the opener is allowed to modify the about:blank document until the target URL starts loading. For pages that require basic authentication, Firefox will display the authentication dialog while remaining at about:blank.

This behavior allows the opener to append a script to that page that will show a Javascript dialog when the basic-authentication dialog is displayed. After the authentication dialog is closed, the script can redirect the page to any other domain and the Javascript dialog will remain active in that window.

Finally, the malicious code is loaded from a data URI in an Iframe, so the Javascript dialog will show "This page says" in its title instead of the URL of the attacker's page.

Passing the credentials directly to the URL (as in https://[e-mail]) shows a different dialog that also works for the attack. Perhaps there are other types of dialogs that are vulnerable to this.

Video: https://youtu.be/Sk6tAGEMH4I

POC is attached.

This is similar to Bug 1313916.

Source code:

<meta charset="UTF-8">
<a href="https://jigsaw.w3.org/HTTP/Basic/" onclick="e(href); return false;">Click here</a>
<script>
function e(url){
var x = open(url,'','width=800,height=400');
var s = document.createElement('script');
s.src = 'data:text/plain,function checkFocus(){fetch("data:,").then(()=>{document.hasFocus()?checkFocus():(alert("Arbitrary message 🙂"),location.replace("https://jigsaw.w3.org"))})}document.title="Google",checkFocus();';
setTimeout(() => {x.document.body.appendChild(s);},100);
}
</script>

Comment 1

[Ademar Nowasky Junior]

Attachment: index-fixed.html

Comment 2

[Ademar Nowasky Junior]

Firefox 89.0.2 and 90.0b12 are vulnerable. Tested on Windows.

Comment 3

[Ademar Nowasky Junior]

Attachment: poc2.html

Attached is a new PoC that doesn't rely on the HTTP authentication dialog.

In this PoC the location is set to a website that takes some seconds to respond. During this time alert() is called and subsequent alerts are called through location=javascript:alert(). Lastly, a new location is set to the target website.

In order for the alerts to remain active after navigation, the user must close the first alert before the slow server responds.

Comment 4

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

esr78 doesn't seem to have the issue. It closes the alert() when loading mozilla.org.

Comment 5

[Andrew McCreight [:mccr8]]

I did a bisection via this command: ./mach mozregression --good 78 --pref "fission.autostart:false" --arg <path to poc2.html>

In the good case, when the page navigates to hixie.ch, the dialog goes away, then we navigate to Mozilla. However, the bad behavior I'm seeing is slightly different than described. The page navigates to hixie.ch, while the dialog remains, but we never do the navigation to Mozilla. That still seems bad, because we're on a different web page than where the dialog is from, but maybe it limits the attack a bit because you can only end up on websites that respond slowly? Or something?

Anyways, here's the regression range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=cb79af30c8e5fc273d09eccea39d84910c3464aa&tochange=0f5e4a3c6f0a6ab0fcd77c46cedc819e2bd0b3f8

I'll take a look at these, but it isn't obvious to me which might be at fault.

Comment 6

[Andrew McCreight [:mccr8]]

Maybe bug 1681249 is at fault. It involves navigation, but it talks about custom protocols so maybe not.

There's also a number of patches by Micah Tigley relating to content prompts which also seems suspicious, so maybe bug 1680637 or one of the others is at fault. Micah, does this regression look like it could be related to your patches? Thanks.

Comment 7

[Ademar Nowasky Junior]

(In reply to Andrew McCreight [:mccr8] from comment #5)

However, the bad behavior I'm seeing is slightly different than described. The page navigates to hixie.ch, while the dialog remains, but we never do the navigation to Mozilla. That still seems bad, because we're on a different web page than where the dialog is from, but maybe it limits the attack a bit because you can only end up on websites that respond slowly? Or something?

Hi Andrew,

Are you also testing on Windows? This also happens for you with the index-fixed.html (the navigation goes to https://jigsaw.w3.org/HTTP/Basic/ instead of going to https://jigsaw.w3.org/) ?

Here's a video on how the poc2.html behaves in my system: https://youtu.be/1eDO35biTz8

Comment 8

[Andrew McCreight [:mccr8]]

(In reply to Ademar Nowasky Junior from comment #7)

Are you also testing on Windows?

I'm on OSX. I'm not sure why this would be system specific, but that could be the issue.

This also happens for you with the index-fixed.html (the navigation goes to https://jigsaw.w3.org/HTTP/Basic/ instead of going to https://jigsaw.w3.org/) ?

With index-fixed, it goes to the Basic/ site and there is no dialog window (it can be seen very briefly, but only for an instant).

Here's a video on how the poc2.html behaves in my system: https://youtu.be/1eDO35biTz8

Thanks. Yeah, I'm not seeing that behavior.

Comment 9

[:Gijs (he/him)]

Let's use Micah's moco address for the needinfo. But yeah, it's likely that the content prompt changes from bug 1680637 and friends is related here...

Comment 10

[Micah [:mtigley] (she/her)]

The changes from bug1680637, specifically: https://searchfox.org/mozilla-central/rev/538569c63ffb17d590c51df66f9fff790dd7431e/browser/base/content/browser.js#9317-9333 , are likely related to these prompts persisting on a redirect. I would think the prompts would be closed onLocationChange, but it seems this isn't triggered using the above test cases. In general, we need to make sure abortDialogs() is called on a redirect.

Comment 11

[Micah [:mtigley] (she/her)]

(In reply to Andrew McCreight [:mccr8] from comment #5)

the bad behavior I'm seeing is slightly different than described. The page navigates to hixie.ch, while the dialog remains, but we never do the navigation to Mozilla.

I can see this behavior too with the old content prompts too. You can set the pref prompts.contentPromptSubDialog to false to get the old ones. The only difference is the old prompts are dismissed when the page finally navigates to hixie.ch, but still no navigation to Mozilla.

(In reply to Micah [:mtigley] (she/her) from comment #10)

The changes from bug1680637, specifically: https://searchfox.org/mozilla-central/rev/538569c63ffb17d590c51df66f9fff790dd7431e/browser/base/content/browser.js#9317-9333 , are likely related to these prompts persisting on a redirect. I would think the prompts would be closed onLocationChange, but it seems this isn't triggered using the above test cases. In general, we need to make sure abortDialogs() is called on a redirect.

It looks like we'd need make sure TabDialogBox.onLocationChange is also called from the tabbrowser's onLocationChange . We'd need to do something like:

let tabDialogBox = gBrowser.getTabDialogBox();
tabDialogBox.onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags);

This should fix the prompts persisting on a redirect, but I don't think it addresses why the navgiation to Mozilla never happens.

Comment 12

[Ademar Nowasky Junior]

Today my Firefox Developer Edition has updated from version 92.0b1 to 92.0b2 and now I'm experiencing the same behavior described by Andrew where in poc2 the navigation doesn't redirect to mozilla.org but are still active at hixie.ch.

In Firefox 91.0 the issue behaves as initially reported.

Comment 13

[Ademar Nowasky Junior]

Attachment: poc3.html

Of course this new behavior where we never reach mozilla.org could be bypassed by having the attacker's website (in this case hixie.ch) make the redirect.

In the attached poc3.html, hixie.ch will make the redirect using a meta tag, resulting in the javascript dialogs being active at mozilla.org.

Comment 14

[Andrew McCreight [:mccr8]]

Thanks for the updated test case. Yes, that navigates to Mozilla for me now.

Comment 15

[Andrew McCreight [:mccr8]]

I'll move this back to Firefox because it seems to be a regression from a front end patch.

Comment 16

[:Gijs (he/him)]

(In reply to Micah [:mtigley] (she/her) from comment #11)

It looks like we'd need make sure TabDialogBox.onLocationChange is also called from the tabbrowser's onLocationChange . We'd need to do something like:

let tabDialogBox = gBrowser.getTabDialogBox();
tabDialogBox.onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags);

This should fix the prompts persisting on a redirect, but I don't think it addresses why the navgiation to Mozilla never happens.

This would fix it, but that would only help for tabbed browsers, and prompts can appear in other places (e.g. webextension sidebars and popups and so on).

We register the progress listener on the browser directly, at https://searchfox.org/mozilla-central/rev/3a8091d1c29473a0839ad7a5810028f41363fe2e/browser/base/content/browser.js#9244 .

The real question is - why isn't that progress listener being called by the browser element? Micah, do you have time to investigate that further? We can also try to look at this together tomorrow if you prefer.

Comment 17

[:Gijs (he/him)]

(In reply to Ademar Nowasky Junior from comment #12)

Today my Firefox Developer Edition has updated from version 92.0b1 to 92.0b2 and now I'm experiencing the same behavior described by Andrew where in poc2 the navigation doesn't redirect to mozilla.org but are still active at hixie.ch.

As an aside, Andrew, is this something we should investigate in a separate, non-security bug? I assume it is unrelated to the prompts...

Comment 18

[Andrew McCreight [:mccr8]]

Sure. I have no idea if it is expected or not or what might be going wrong.

Comment 19

[:Gijs (he/him)]

Christoph/Freddy, can you help explain why the assignments to location in the script in the iframe load in a separate tab from the iframe in which they run? And why, even if they required a new browsing context, they then end up reusing that browsing context, instead of the 3 assignments creating 3 different contexts?

Comment 20

[Ademar Nowasky Junior]

I'm giving my two cents here:

(In reply to :Gijs (he/him) from comment #19)

why the assignments to location in the script in the iframe load in a separate tab from the iframe in which they run?
Because the iframe opens a new tab through window.open and the new tab is the one that has the javascript code to change its location.

(In reply to :Gijs (he/him) from comment #19)

And why, even if they required a new browsing context, they then end up reusing that browsing context, instead of the 3 assignments creating 3 different contexts?
Giving that a new tab was opened and it contains assignments to its location, it will reuse the same browsing context.

Comment 21

[Ademar Nowasky Junior]

sorry... messed the last response with the quote

(In reply to :Gijs (he/him) from comment #19)

And why, even if they required a new browsing context, they then end up reusing that browsing context, instead of the 3 assignments creating 3 different contexts?

Giving that a new tab was opened and it contains assignments to its location, it will reuse the same browsing context.

Comment 22

[Ademar Nowasky Junior]

my god... trying again:

(In reply to :Gijs (he/him) from comment #19)

Christoph/Freddy, can you help explain why the assignments to location in the script in the iframe load in a separate tab from the iframe in which they run?

Because the iframe opens a new tab through window.open and the new tab is the one that has the javascript code to change its location.

(In reply to :Gijs (he/him) from comment #19)

And why, even if they required a new browsing context, they then end up reusing that browsing context, instead of the 3 assignments creating 3 different contexts?

Giving that a new tab was opened and it contains assignments to its location, it will reuse the same browsing context.

Comment 23

[:Gijs (he/him)]

Gah, I can't believe I missed the open call. Thanks for clarifying!

Comment 24

[:Gijs (he/him)]

Damnit. This is one of those bugs where the investigation takes forever and then finally the problem it reveals is really dumb.

Long story short, yes we have a web progress listener, and yes it gets added correctly... and then it gets removed again. I went down a lot of dead ends assuming things about what might cause it to be removed (process flip that destroys the browsing context, weak references and nothing holding on to it, weird indirection that we use for tabbed browser listeners but not elsewhere... a bunch).

None of that made any difference so then I just started adding MOZ_LOG stuff to BrowsingContextWebProgress.cpp to work out wtf was happening to the listener, and then when I saw where it got removed, added stack tracing to that. That turned out to make short work of things:

0 removeProgressListener(aListener = "[xpconnect wrapped nsIWebProgress]") ["chrome://global/content/elements/browser-custom-element.js":857:23]
1 _onLastDialogClose() ["chrome://browser/content/browser.js":9264:17]
2 closingCallback(event = "[object CustomEvent]") ["chrome://browser/content/browser.js":9205:13]
3 open/this._closingCallback(args = "[object CustomEvent]") ["resource://gre/modules/SubDialog.jsm":139:24]
4 close(aEvent = "[object CustomEvent]") ["resource://gre/modules/SubDialog.jsm":231:30]
5 _onContentLoaded/this._frame.contentWindow.close() ["resource://gre/modules/SubDialog.jsm":414:11]
6 abort() ["resource://gre/modules/SubDialog.jsm":211:30]
7 abortDialogs/<(dialog = "[object Object]", "1", "[object Object],[object Object]") ["resource://gre/modules/SubDialog.jsm":1017:60]
8 forEach(callbackfn = [function]) ["self-hosted":205:21]
9 abortDialogs(filterFn = "undefined") ["resource://gre/modules/SubDialog.jsm":1017:35]
10 onLocationChange(aWebProgress = "[xpconnect wrapped nsIWebProgress]", aRequest = "[xpconnect wrapped nsIRequest]", aLocation = "[xpconnect wrapped nsIURI]", aFlags = "0") ["chrome://browser/content/browser.js":9345:44]

#01: mozilla::dom::BrowsingContextWebProgress::RemoveProgressListener(nsIWebProgressListener*)[opt/toolkit/library/build/XUL +0x3bcd441]
#02: NS_InvokeByIndex[opt/toolkit/library/build/XUL +0x13718e]
#03: xpt::detail::sParams[opt/toolkit/library/build/XUL +0x6149ed0]

So I'm staring at this stack for _onLastDialogClose going... "but this isn't the last dialog?!"

Well, the code looks like this:

    let hasDialogs =
      this._tabDialogManager.hasDialogs ||
      this._contentDialogManager?.hasDialogs;

    if (!hasDialogs) {
      this._onFirstDialogOpen();
    }

    let closingCallback = event => {
      if (!hasDialogs) {
        this._onLastDialogClose();
      }
      ...

so we're caching whether there are dialogs, and because for the first dialog we open, there aren't (yet) any of them, we store false, and when that first dialog closes, we assume it's the last dialog, and then in _onLastDialogClose, we "clean up" the progress listener. Prematurely, as it turns out...

So the patch is trivial, now I just need an automated test...

Comment 25

[:Gijs (he/him)]

Attachment: Bug 1718571, r?mtigley!,pbz!

Comment 26

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #24)

    let hasDialogs =
      this._tabDialogManager.hasDialogs ||
      this._contentDialogManager?.hasDialogs;

    if (!hasDialogs) {
      this._onFirstDialogOpen();
    }

    let closingCallback = event => {
      if (!hasDialogs) {
        this._onLastDialogClose();
      }
      ...

Oh, and to note: this doesn't just hit any page that shows more than 1 dialog, because ordinarily, when the second dialog fires, hasDialogs is once again false (we closed the first dialog!) and so we just re-run onFirstDialogOpen and re-add a progress listener and everything is hunky-dory. The problem occurs when a page manages to queue up 2 or more dialogs immediately. This doesn't normally happen because a call to alert will stop the JS that's executing at that point until the call returns (ie the dialog is closed), and while the dialog is up we enter modal state in the document in which the dialog is up (so timeouts and events shouldn't fire until after the dialog is removed again). So the next alert() won't hit this code until we've closed the previous dialog. The PoC works around this by causing the dialogs to be opened from a different context, which isn't frozen in that modal state, thus allowing it to queue up more than 1 dialog before the 1st dialog closes.

I wonder if we should extend the modal state to the entire group of browsing contexts... Nika/Olli, thoughts on that?

Comment 27

[Nika Layzell [:nika] (ni? for response)]

I believe at one point we were considering doing that but we only did it partially, and :smaug had to revert us back to the old behaviour for consistency. I definitely think we want to eventually make modal state a bit more clever.

That being said, there are probably ways to get code to run when a BC is supposed to be suspended in a modal state, and we shouldn't be relying on perfect blocking of scripts.

Comment 28

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

We've been thinking about extending user input suppression to cover the whole bc group, but perhaps less about modal state.
Anyhow, that is tricky : what should happen if some, for example, xhr.onload triggers alert() when another modal dialog is shown?

Comment 29

[:Gijs (he/him)]

(In reply to Olli Pettay [:smaug] from comment #28)

We've been thinking about extending user input suppression to cover the whole bc group, but perhaps less about modal state.
Anyhow, that is tricky : what should happen if some, for example, xhr.onload triggers alert() when another modal dialog is shown?

I would have assumed that we stop dispatching events, so the onload would be queued until the group is no longer in modal state. But I'm not even 1% as familiar with the web standards involved as you are so I'm probably missing something obvious about why that wouldn't work?

Comment 30

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

Yes, that is how it might work, it is just that one would need to implement that queuing for everything which might trigger JS to run: network callbacks, various postMessage cases, geolocation, notifications, etc.

Comment 31

[:Gijs (he/him)]

Landed the patch part on autoland: https://hg.mozilla.org/integration/autoland/rev/9c352dc6eafc997627c1ed4c8629f32e285207f1

Filed bug 1735314 to land tests after that patch has made it to release etc.

Comment 32

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/9c352dc6eafc

Comment 33

[:Gijs (he/him)]

Comment on attachment 9244608 [details]
Bug 1718571, r?mtigley!,pbz!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-moderate
• User impact if declined: User confusion / data exfiltration via spoofing
• Fix Landed on Version: 95, uplift to 94
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very small JS change to avoid cached information causing logical errors
• String or UUID changes made by this patch: None

Beta/Release Uplift Approval Request

• User impact if declined:
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1. Open attachment 9235868 [details]

2. Click the link
3. close the first dialog by clicking OK
4. wait for mozilla.org to load

ER: dialogs get closed when a different site loads

AR (before patch): dialog stays visible

• List of other uplifts needed: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very small JS change to avoid cached information causing logical errors
• String changes made/needed: None

Comment 34

[Bogdan Maris, Desktop Test Engineering]

Reproduced the initial issue using the steps from comment 33 on an old Nightly build from 2021-10-11. Verified that the dialog closed correctly after the new site loaded using latest Nightly build across platforms (Windows 10, Ubuntu 18.04 and macOS 11.6).

Comment 35

[Dianna Smith [:diannaS]]

Comment on attachment 9244608 [details]
Bug 1718571, r?mtigley!,pbz!

Approved for 94.0b6

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7f84c5121cf1

Comment 37

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9244608 [details]
Bug 1718571, r?mtigley!,pbz!

Approved for 91.3esr.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/2d31bb84822b

Comment 39

[Bogdan Maris, Desktop Test Engineering]

Also verified using 94.0b6 and latest esr91 from treeherder across platforms (Windows 10, macOS 11.6 and Ubuntu 18.04).

Comment 40

[Tom Ritter [:tjr]]

Attachment: advisory.txt