Private Browsing tabs can open in normal browsing during PBM teardown
Categories
(Firefox :: Private Browsing, defect, P2)
Tracking
()
People
(Reporter: marufbinmurtuza, Assigned: emz)
References
Details
(Keywords: csectype-spoof, reporter-external, sec-low, Whiteboard: [adv-main135+][adv-ESR128.7+])
Attachments
(7 files, 1 obsolete file)
|
464 bytes,
text/html
|
Details | |
|
10.91 KB,
text/plain
|
Details | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
phab-bot
:
approval-mozilla-beta+
|
Details | Review |
|
48 bytes,
text/x-phabricator-request
|
phab-bot
:
approval-mozilla-esr128+
|
Details | Review |
|
1.94 MB,
image/gif
|
Details | |
|
245 bytes,
text/plain
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:132.0) Gecko/20100101 Firefox/132.0
Steps to reproduce:
Open a Incoginto Tab. Log into to a website and replace the website url in the following HTML. Then, load the HTML in a new incognito tab and allow it to open multiple tabs. Close the Incognito Browser before the tab opening loop ends.
POC CODE: (Screen recording on Google Drive.)
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Tabblast</title>
<script>
window.onload = () => {
const currentURL = "https://stackoverflow.com/";
for (let i = 0; i < 50000000; i++) {
window.open(currentURL, '_blank');
}
};
</script>
</head>
<body>
</body>
</html>
Actual results:
Incognito Mode tabs shifted to Main Browser with session tokens. But those tabs aren't logged in the browser history.
Expected results:
The incognito tabs shouldn't be moved to Main browser. And if it goes to the main browser, then it should be logged in the browser history.
| Reporter | ||
Comment 1•1 year ago
|
||
| Assignee | ||
Updated•1 year ago
|
| Reporter | ||
Comment 3•1 year ago
|
||
Hello guys,
I’m new to bugzilla.
So, I didn’t know that it has a different form for bug bounty. So, I emailed you as soon as I learned about that.
Updated•1 year ago
|
Updated•1 year ago
|
| Assignee | ||
Updated•1 year ago
|
Comment 4•1 year ago
|
||
Comment 5•1 year ago
|
||
From the movie it appears that these are all still private tabs, it's just that we're confused about which window to attach them to and they aren't marked as such. Just because you close a tab or window it doesn't immediately kill a running loop. I don't know why we still allow those loops to do UI-based things like alert() and window.open() though -- seems like we should be able to know they don't have a home to be launched from and just return errors instead.
Paul: Am I wrong? when you marked it "S2" could you tell that the new tabs were no longer private? Or is it S2 because it will freak users out regardless? It's certainly confusing UI, but it does depend on the user closing the private browsing window themselves.
Updated•1 year ago
|
| Assignee | ||
Comment 6•1 year ago
•
|
||
| Assignee | ||
Comment 7•1 year ago
|
||
On Nightly on Windows I also get a crash: https://crash-stats.mozilla.org/report/index/e81d562d-b832-4dcd-8154-e74570241205
| Assignee | ||
Comment 8•1 year ago
|
||
(In reply to Daniel Veditz [:dveditz] from comment #5)
From the movie it appears that these are all still private tabs, it's just that we're confused about which window to attach them to and they aren't marked as such. Just because you close a tab or window it doesn't immediately kill a running loop. I don't know why we still allow those loops to do UI-based things like alert() and window.open() though -- seems like we should be able to know they don't have a home to be launched from and just return errors instead.
Paul: Am I wrong? when you marked it "S2" could you tell that the new tabs were no longer private? Or is it S2 because it will freak users out regardless? It's certainly confusing UI, but it does depend on the user closing the private browsing window themselves.
Sorry I realize I haven't really answered your question. It's hard to tell if those tabs are still private because I can't reproduce. For me it crashes (IMO correctly). I suspect that they are still private, so I'm mostly concerned about users confusing normal browsing and private browsing.
If you have time, could you try to reproduce? Even if we can't we can probably still fix the crash issue and that would hopefully fix what the reporter is describing.
| Assignee | ||
Comment 9•1 year ago
|
||
I was able to reproduce myself on Windows 11 (ARM) with Firefox 133.0. We don't seem to crash there and the tabs get opened in the normal browsing window. They still appear to be private browsing tabs though. When the site sets a cookie it gets set for privateBrowsingId: 1 as expected.
| Assignee | ||
Comment 10•1 year ago
|
||
Nika, I think I found a source for the crashes reported in Bug 1831432. It doesn't seem to crash in Release for some reason, but in local builds and on Nightly I can reliably crash the browser using the instructions from comment 0.
I've attached a debugger and we do indeed hit the line you suspected in Bug 1831432: https://searchfox.org/mozilla-central/rev/5ad94327fc03d0e5bc6aa81c0d7d113c2dccf947/dom/ipc/ContentParent.cpp#5260 where we end up falling back to a normal browsing window. We do this even though aOriginAttributes.mPrivateBrowsingId == 1.
What would you suggest is the correct behavior here? Since the PBM window has been closed could we just bail out of opening the window, or is that too risky, because that could impact other scenarios? Alternatively, should we open a new PBM window?
| Assignee | ||
Comment 11•1 year ago
|
||
I'm reducing severity because a) the tabs are still isolated from normal browsing and b) its quite complicated to hit this. It requires the user to allowlist a site from the popup blocker.
Comment 12•1 year ago
|
||
(In reply to Paul Zühlcke [:pbz] from comment #10)
Nika, I think I found a source for the crashes reported in Bug 1831432. It doesn't seem to crash in Release for some reason, but in local builds and on Nightly I can reliably crash the browser using the instructions from comment 0.
I've attached a debugger and we do indeed hit the line you suspected in Bug 1831432: https://searchfox.org/mozilla-central/rev/5ad94327fc03d0e5bc6aa81c0d7d113c2dccf947/dom/ipc/ContentParent.cpp#5260 where we end up falling back to a normal browsing window. We do this even though
aOriginAttributes.mPrivateBrowsingId == 1.What would you suggest is the correct behavior here? Since the PBM window has been closed could we just bail out of opening the window, or is that too risky, because that could impact other scenarios? Alternatively, should we open a new PBM window?
You might be able to get away with adding a private browsing check to the if (!outerWin) { check here: https://searchfox.org/mozilla-central/rev/5ad94327fc03d0e5bc6aa81c0d7d113c2dccf947/dom/ipc/ContentParent.cpp#5257-5264. If this block is skipped, we'll fail to set outerWin, which means we should fall back to opening a brand new browser window, which I think should be OK here.
Alternatively, we could also fail like we do below (https://searchfox.org/mozilla-central/rev/5ad94327fc03d0e5bc6aa81c0d7d113c2dccf947/dom/ipc/ContentParent.cpp#5257-5264), which would also be OK - preventing the window from successfully opening in this edge case (this may be preferable if we're already ending the PB session - as we wouldn't necessarily want the new window to carry over into a new PB session I suppose?)
| Assignee | ||
Comment 13•1 year ago
|
||
| Assignee | ||
Comment 14•1 year ago
|
||
The patch is ready. I'm holding off from landing until we have a sec rating.
Updated•11 months ago
|
Comment 15•11 months ago
|
||
Comment 16•11 months ago
|
||
| Reporter | ||
Comment 17•11 months ago
|
||
Hello,
Can you guys please confirm when my name will be listed on the credit page?
Updated•11 months ago
|
Comment 18•11 months ago
|
||
(In reply to Maruf Bin Murtuza from comment #17)
Hello,
Can you guys please confirm when my name will be listed on the credit page?
It's not clear to me what you're referring to - if a list of security fixes, presumably not before the fix reaches release and we write release notes for that release - it only just landed in our nightly branch.
Comment 19•11 months ago
|
||
The patch landed in nightly and beta is affected.
:emz, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox135towontfix.
For more information, please visit BugBot documentation.
| Assignee | ||
Comment 20•11 months ago
|
||
Original Revision: https://phabricator.services.mozilla.com/D231928
Updated•11 months ago
|
Comment 21•11 months ago
|
||
beta Uplift Approval Request
- User impact if declined: Rare chance of a race condition that leads to private browsing tabs being opened in normal browsing windows. Since this behavior is undefined it may lead to further privacy / security issues.
- Code covered by automated testing: no
- Fix verified in Nightly: no
- Needs manual QE test: yes
- Steps to reproduce for manual QE testing: See bug STR
- Risk associated with taking this patch: low
- Explanation of risk level: Simple code change.
- String changes made/needed: no
- Is Android affected?: yes
| Assignee | ||
Comment 22•11 months ago
|
||
Original Revision: https://phabricator.services.mozilla.com/D231928
Updated•11 months ago
|
Comment 23•11 months ago
|
||
esr128 Uplift Approval Request
- User impact if declined: Rare chance of a race condition that leads to private browsing tabs being opened in normal browsing windows. Since this behavior is undefined it may lead to further privacy / security issues.
- Code covered by automated testing: no
- Fix verified in Nightly: no
- Needs manual QE test: yes
- Steps to reproduce for manual QE testing: See bug STR
- Risk associated with taking this patch: low
- Explanation of risk level: Simple code change.
- String changes made/needed: no
- Is Android affected?: yes
| Assignee | ||
Comment 24•11 months ago
|
||
I haven't tested this on Android, but seems the affected code is part of Gecko it could be affected. There may also be other ways to trigger the bug which we are not aware of. You basically have to find a way to trigger a window open during which the parent PBM window gets closed / dies.
Updated•11 months ago
|
Comment 25•11 months ago
|
||
| uplift | ||
Updated•11 months ago
|
Updated•11 months ago
|
Comment 26•11 months ago
|
||
| uplift | ||
Updated•11 months ago
|
Updated•11 months ago
|
Comment 27•11 months ago
•
|
||
Reproduced the issue with Firefox 134.0.1 on Window 10x64 by following the steps from comment 0 and video from comment 0. After closing the Private browsing window while the tabs are loading the tabs will get loaded in the normal window and the user will be signed in in the normal browsing.
The issue is no longer reproducible with Firefox 136.0a1 (2025-01-21), 136.0b7, and 128.7.0esr (20250118171327) on Windows 10x64, macOS 12, and Ubuntu 24.04.
However, if I open a new private window and go to the website again, I will still signed in to the website (see attached screen recording). Should we reopen this issue or file a new one?
Note that I have replaced the website in the attached test case as well since I cannot log in to StackOverflow for some reason.
| Assignee | ||
Comment 28•11 months ago
|
||
Thank you! Oh, that's odd. Let's track it as a separate bug.
Comment 29•11 months ago
|
||
(In reply to Emma Zühlcke [:emz] from comment #28)
Thank you! Oh, that's odd. Let's track it as a separate bug.
Thank you! I have filed bug 1942988. I am closing this as verified per comment 27 since this particular issue is no longer reproducible.
Updated•11 months ago
|
Updated•11 months ago
|
| Reporter | ||
Comment 30•11 months ago
|
||
Hello there,
I hope this message finds you well. I would like to inquire about the process for receiving the bounty. Additionally, could you please let me know the expected timeline for the payment?
Updated•10 months ago
|
Updated•10 months ago
|
Comment 31•10 months ago
|
||
Updated•10 months ago
|
Updated•6 months ago
|
Description
•