Closed Bug 1839464 (CVE-2023-3482) Opened 1 year ago Closed 1 year ago

"Block all cookies" bypass for localstorage using about:blank iframe, plus document.cookie weirdness

Categories

(Core :: Privacy: Anti-Tracking, defect, P2)

defect

Tracking

()

VERIFIED FIXED
116 Branch
Tracking Status
firefox-esr102 --- wontfix
firefox-esr115 115+ verified
firefox114 --- wontfix
firefox115 + verified
firefox116 + verified

People

(Reporter: textshell, Assigned: timhuang)

Details

(4 keywords, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main115+])

Attachments

(5 files)

Attached file testfiles.zip

[originally submitted via email, but the autoresponder asked to resubmit via bugzilla]
While testing a cookie auto deletion addon, i came across what seem to be a cookie/storage permission bypass.

Steps to reproduce:

  1. As usual serve the attached files using a local webserver (like python3 -mhttp.server) and access them on a
    non localhost ip.

  2. I have tested this with a fresh firefox nightly (116.0a1 (2023-06-16)(64bit).

  3. In the fresh profile set cookie blocking to "custom" and select to block "All cookies".[1]

  4. Confirm that normal access to local storage is blocked by using the attached test-noiframe.html
    3a: Access to the storage APIs gives a "DOMException: The operation is insecure.", the storage tab in developer tools show no saved data.

  5. But Using the attached test2.html give unblocked access to local storage.

Expected Result:
Firefox blocks storage access the same as when using the non iframe version of the test. No data stored.

Actual Result:
The storage apis work and ignore the setting to block the access:

  • No exception on usage
  • When entering data in the textbox and pressing save, on reloading the page, the page can retrieve the data.
  • The saved data is not visible in developer tools, but even after restarting firefox the page can still read back the data.
  • The origin with the saved data is shown in "Manage Cookies and Site Data" as having used storage.

I have not tracked done the regression point, but using firefox 100, the blocking works and the api access gives the expected exceptions.
This seems to be a recent regression.

The core part of test2.html is as follows. It seems by just using the contentWindow of a otherwise defaulted iframe,
the blocking of the storage api is evaded.

<iframe id="preview" name="preview"></iframe>
<script>
let p = document.getElementById('preview');
let currentData = p.contentWindow.localStorage.getItem('blah');
document.getElementById('odata').innerText = currentData;

function setTest() {
    p.contentWindow.localStorage.setItem('blah', document.getElementById('idata').value);
}
</script>

From looking at the profile directory the local storage seems to be attributed to the origin used to access test2.html.

storage.sqlite
INSERT INTO origin VALUES(2,'','192.168.123.123','http://192.168.123.123:4567','L9',9,1686945337316151,1,0);

storage/default/http+++192.168.123.123+4567/ls/data.sqlite
INSERT INTO "database" VALUES('http://192.168.123.123:4567',9,1686944680773745,0,6144);

I've played around a bit if this also allows evading storage isolation, but i did not the storage to leak across.
But this still evades supported data privacy settings.

[1] The problem also manifests with other global settings and blocking the specific origin.

Flags: sec-bounty?
Group: firefox-core-security → dom-core-security
Component: Security → Storage: localStorage & sessionStorage
Product: Firefox → Core

Paul, Tim: is "Storage" the right component for this, or is "Privacy: anti-tracking" better?

Flags: needinfo?(tihuang)
Flags: needinfo?(pbz)
Attached file test2.html

From looking at the profile directory the local storage seems to be attributed to the origin used to access test2.html.

A frame (or window.open()) with no source URL contains an "about:blank" document, which inherits the origin of the browsing context that created it. The "same origin policy" means that If the frame were not the same origin, when your code tried to access anything inside the frame it would have have thrown security errors. "p" is OK (the frame element is part of the outer document), "p.contentWindow" wouldn't have worked

Interesting. Similarly, p.contentDocument.cookie or p.contentWindow.document.cookie can be used to set cookies but I'm not sure where they live. When you switch the cookie setting back to normal (and refresh the testcase to pick up the new settings) those cookies are gone. If you switch back to blocking all cookies and refresh, the cookie you set on the frame's document.cookie is back again. At least as seen by the document.cookie API. It doesn't get sent to the site nor does it show up in devtools cookie storage either way the cookie blocking is set.

Since cookies are a different group than storage but the DOM level confusion is similar I think anti-tracking is probably a better place to start. There's some confusion wrt that "about:blank" frame. Is some code checking against the URL and getting a null host rather than checking the principal for the true origin?

Status: UNCONFIRMED → NEW
Component: Storage: localStorage & sessionStorage → Privacy: Anti-Tracking
Ever confirmed: true
Summary: Local storage access block bypass using iframe → "Block all cookies" bypass for localstorage using about:blank iframe, plus document.cookie weirdness
Attached file testcookie.html

Yes, the cookie behavior is strange.

I'm testing with

let p = document.getElementById('preview');
let currentData = p.contentWindow.document.cookie
document.getElementById('odata').innerText = currentData;

function setTest() {
    p.contentWindow.document.cookie = 'blah=' + document.getElementById('idata').value + '; expires=Mon, 01 Jan 2024 00:00:00 GMT';
}

In contrast to local storage, setting a new cookie case seem to get blocked by setting an explicit "block" exception for the origin.
And the cookie data disappears when i use about:processes to kill the content process of the tab. Testing further using 2 tabs the cookie jar seems to have different storage for each tab/content process (each tab can set a different value and read it back on page reload).
And using the "clear recent history" feature selecting "today" does not clear the cookie, but selecting "everything" does clear it.

So the cookie code seems to do different checks in different code paths and so has more than one behavior. And its behavior is different to the detailed behavior with storage.

Tim is looking into this.

Flags: needinfo?(pbz)
Flags: needinfo?(tihuang)

The bypass of cookie blocking is because we exempt the storage access check for the "about:blank" iframe due to here. So, we will get StorageAccess::eAllow even in the setting with blocking all cookies. I think this is an oversight of the storage access check for about:blank iframe.

After talking with Ben about this issue, we think we should restrict more storage access checks on about pages. We think we should still do storage access checks for content exposed about pages.

Assignee: nobody → tihuang
Severity: -- → S2
Status: NEW → ASSIGNED
Priority: -- → P2

This bug has been marked as a regression. Setting status flag for Nightly to affected.

I've applied the patch to a slightly older checkout of the release branch (around 114), and i can no longer reproduce the problem with the patch applied.

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 116 Branch

Looks like this affects all branches. Can we assign a sec rating to this bug please?

Relatedly, this should have gotten sec-approval before landing.

Very odd: Searchfox gives blame to bug 1765313 for touching the code pointed at in comment 9 that you patched in D181807. That would indicate it regressed in Fx103, which is consistent with the reporter's statement that Firefox 100 was fine. It would also mean Firefox 102 was unaffected.

But I don't see those lines touched in the patches for bug 1765313. Nor was it in the patch as actually checked in to mozilla-centra so it's hard to claim a check-in merge introduced phantom blame. I see the same code in ESR 91 blamed on bug 1612378.
https://searchfox.org/mozilla-esr91/source/toolkit/components/antitracking/StorageAccess.cpp#154-157

Is something else interacting with this that made the reporter experience OK behavior in Firefox 100? Maybe this code was there, but the isolation was off by default?

I'm unhappy with these custom scheme checks and code that has to have deep knowledge of protocols (such as checking a list of specific URLs or matches and pulling out a couple that are causing problems. That means when we add more such URLs or protocols, or someone writes new code that needs to behave like this it will potentially break in unsafe ways. That's the whole point of using principal comparisons and bundling knowledge into protocol flags (and even "dynamic" protocol flags for schemes like "about" where different URLs have different properties).

Looks like the TCP roll-out was in Firefox 103, and I assume this bug doesn't happen without partitioning enabled. Some folks might have stumbled on it slightly earlier and manually enabled it in about:config but that's not going to be a huge number of people.

We should definitely get this fix into Firefox 115 based on it's privacy severity, and because that will be the next ESR. We don't need to go out of our way to fix ESR-102, though it looks like it might apply pretty cleanly so I wouldn't object to it.

:timhuang could you please add a release uplift request on this?

Comment on attachment 9340567 [details]
Bug 1839464 - Check storage access in content accessible about pages. r?bvandersloot!

Beta/Release Uplift Approval Request

  • User impact if declined: The iframe can access the storage/cookies even if the user disables cookie/storage access.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The code change of the patch is small.
  • String changes made/needed: none
  • Is Android affected?: Yes
Flags: needinfo?(tihuang)
Attachment #9340567 - Flags: approval-mozilla-release?

Do I need to request another uplift for beta, or is the release uplift enough?

The patch landed in nightly and beta is affected.
:timhuang, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox115 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(tihuang)

(In reply to Tim Huang[:timhuang] from comment #20)

Do I need to request another uplift for beta, or is the release uplift enough?

Release is enough, 115 is now in release and there's where the uplift needs to happen

Flags: needinfo?(tihuang)
Flags: sec-bounty? → sec-bounty+
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][adv-main115+]
Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main115+] → [reporter-external] [client-bounty-form] [verif?][adv-main115+][adv-esr102.13+]
Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main115+][adv-esr102.13+] → [reporter-external] [client-bounty-form] [verif?]

Comment on attachment 9340567 [details]
Bug 1839464 - Check storage access in content accessible about pages. r?bvandersloot!

Approved for 115.0 RC2

Attachment #9340567 - Flags: approval-mozilla-release? → approval-mozilla-release+
QA Whiteboard: [qa-triaged]
Flags: qe-verify+

I've reproduced this issue with STR from comment 0, on an affected Nightly build (116.0a1, 2023-06-20) with macOS 11.

The issue is verified as fixed on the latest builds, 115.0-build2 RC, 115.0-build2 ESR, and latest Nightly 116.0a1, across platforms: Win 11 x64, macOS 11 and Ubuntu 18.04 x64.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][adv-main115+][adv-esr102.13+]
Alias: CVE-2023-3482
Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main115+][adv-esr102.13+] → [reporter-external] [client-bounty-form] [verif?][adv-main115+]
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: