Javascript dialog remains active in window after redirect
Categories
(Firefox :: General, defect)
Tracking
()
People
(Reporter: nowasky.jr, Assigned: Gijs)
References
(Regression)
Details
(4 keywords, Whiteboard: [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage][adv-main94+][adv-esr91.3+])
Attachments
(5 files, 1 obsolete file)
667 bytes,
text/html
|
Details | |
1.48 KB,
text/html
|
Details | |
790 bytes,
text/html
|
Details | |
48 bytes,
text/x-phabricator-request
|
diannaS
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr91+
|
Details | Review |
318 bytes,
text/plain
|
Details |
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://test@google.com) 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>
Reporter | ||
Comment 1•4 years ago
|
||
Reporter | ||
Comment 2•4 years ago
|
||
Firefox 89.0.2 and 90.0b12 are vulnerable. Tested on Windows.
Reporter | ||
Comment 3•4 years ago
|
||
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.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 4•3 years ago
|
||
esr78 doesn't seem to have the issue. It closes the alert() when loading mozilla.org.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 5•3 years ago
|
||
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•3 years ago
|
||
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.
Reporter | ||
Comment 7•3 years ago
|
||
(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•3 years ago
|
||
(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 tohttps://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.
Assignee | ||
Comment 9•3 years ago
|
||
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•3 years ago
|
||
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•3 years ago
•
|
||
(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 sureabortDialogs()
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.
Reporter | ||
Comment 12•3 years ago
|
||
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.
Reporter | ||
Comment 13•3 years ago
|
||
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•3 years ago
|
||
Thanks for the updated test case. Yes, that navigates to Mozilla for me now.
Comment 15•3 years ago
|
||
I'll move this back to Firefox because it seems to be a regression from a front end patch.
Updated•3 years ago
|
Assignee | ||
Comment 16•3 years ago
|
||
(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.
Assignee | ||
Comment 17•3 years ago
|
||
(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•3 years ago
|
||
Sure. I have no idea if it is expected or not or what might be going wrong.
Assignee | ||
Comment 19•3 years ago
|
||
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 hidden (typo) |
Comment hidden (typo) |
Reporter | ||
Comment 22•3 years ago
|
||
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.
Assignee | ||
Comment 23•3 years ago
|
||
Gah, I can't believe I missed the open
call. Thanks for clarifying!
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 24•3 years ago
|
||
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...
Assignee | ||
Comment 25•3 years ago
|
||
Assignee | ||
Comment 26•3 years ago
|
||
(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?
Assignee | ||
Updated•3 years ago
|
Comment 27•3 years ago
|
||
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.
Updated•3 years ago
|
Comment 28•3 years ago
|
||
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?
Assignee | ||
Comment 29•3 years ago
|
||
(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•3 years ago
|
||
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.
Assignee | ||
Comment 31•3 years ago
|
||
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•3 years ago
|
||
Assignee | ||
Comment 33•3 years ago
|
||
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]
- Click the link
- close the first dialog by clicking OK
- 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
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 34•3 years ago
|
||
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•3 years ago
|
||
Comment on attachment 9244608 [details]
Bug 1718571, r?mtigley!,pbz!
Approved for 94.0b6
Comment 36•3 years ago
|
||
uplift |
Comment 37•3 years ago
|
||
Comment on attachment 9244608 [details]
Bug 1718571, r?mtigley!,pbz!
Approved for 91.3esr.
Comment 38•3 years ago
|
||
uplift |
Comment 39•3 years ago
|
||
Also verified using 94.0b6 and latest esr91 from treeherder across platforms (Windows 10, macOS 11.6 and Ubuntu 18.04).
Updated•3 years ago
|
Comment 40•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Updated•2 years ago
|
Updated•8 months ago
|
Description
•