Closed Bug 1932555 (CVE-2025-1013) Opened 1 year ago Closed 11 months ago

Private Browsing tabs can open in normal browsing during PBM teardown

Categories

(Firefox :: Private Browsing, defect, P2)

Firefox 132
defect

Tracking

()

RESOLVED FIXED
136 Branch
Tracking Status
firefox-esr115 --- wontfix
firefox-esr128 135+ verified
firefox134 --- wontfix
firefox135 + verified
firefox136 + verified

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)

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.

Attached file Proof Of Concept Screen Record & Code (obsolete) —
Component: Untriaged → Private Browsing

Reporter requested sec-bounty? by mail.

Flags: sec-bounty?

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.

Attachment #9439015 - Attachment is obsolete: true
Assignee: nobody → pbz
Severity: -- → S2
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P1

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.

Flags: needinfo?(pbzinwindows)
Flags: needinfo?(pbzinwindows) → needinfo?(pbz)
Sorry I was a bit too fast here, I wanted to assign myself to reproduce and investigate further. I haven't managed to reproduce yet, however I do see a crash when running on debug. Here is the assertion failure + stacktrace. The assertion failure clearly indicates that we're trying to PBM things in a non PBM context. I'll try to reproduce this outside of debug and see if I can get Firefox to stay open.
Flags: needinfo?(pbz)
See Also: → 1831432

(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.

Flags: needinfo?(dveditz)

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.

Flags: needinfo?(dveditz)

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?

Flags: needinfo?(nika)

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.

Severity: S2 → S3
Priority: P1 → P2
Summary: Incognito Tab Escape with session tokens → Private Browsing tabs can open in normal browsing during PBM teardown

(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?)

Flags: needinfo?(nika)
Attached file Bug 1932555, r=nika!

The patch is ready. I'm holding off from landing until we have a sec rating.

Group: firefox-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → 136 Branch

Hello,
Can you guys please confirm when my name will be listed on the credit page?

(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.

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-firefox135 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(emz)
Attachment #9460163 - Flags: approval-mozilla-beta?

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
Flags: qe-verify+
Attachment #9460164 - Flags: approval-mozilla-esr128?

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

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.

Flags: needinfo?(emz)
Attachment #9460163 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9460164 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
QA Whiteboard: [qa-triaged]
Attached image 1932555.gif

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.

Flags: needinfo?(emz)

Thank you! Oh, that's odd. Let's track it as a separate bug.

Flags: needinfo?(emz)

(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.

Flags: sec-bounty? → sec-bounty+

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?

Whiteboard: [adv-main135+]
Whiteboard: [adv-main135+] → [adv-main135+][adv-ESR128.7+]
Attached file advisory.txt
Alias: CVE-2025-1013
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: