1839464 - (CVE-2023-3482) "Block all cookies" bypass for localstorage using about:blank iframe, plus document.cookie weirdness

Status: VERIFIED FIXED

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

Description

Attachment: 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:

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

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

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

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

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

Comment 1

[Daniel Veditz [:dveditz]]

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

Comment 2

[Daniel Veditz [:dveditz]]

Attachment: test-noiframe.html

Comment 3

[Daniel Veditz [:dveditz]]

Attachment: test2.html

Comment 4

[Daniel Veditz [:dveditz]]

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

Comment 5

[Daniel Veditz [:dveditz]]

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?

Comment 6

[Martin Hostettler]

Attachment: testcookie.html

Comment 7

[Martin Hostettler]

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.

Comment 8

[Emma Zühlcke [:emz]]

Tim is looking into this.

Comment 9

[Tim Huang[:timhuang]]

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.

Comment 10

[Tim Huang[:timhuang]]

Attachment: Bug 1839464 - Check storage access in content accessible about pages. r?bvandersloot!

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 12

[Martin Hostettler]

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.

Comment 13

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

Check storage access in content accessible about pages. r=bvandersloot,anti-tracking-reviewers
https://hg.mozilla.org/integration/autoland/rev/d451c1b17cfda3ebedad6d117c5022f2871f1018
https://hg.mozilla.org/mozilla-central/rev/d451c1b17cfd

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Comment 15

[Andrew McCreight [:mccr8]]

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

Comment 16

[Daniel Veditz [:dveditz]]

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

Comment 17

[Daniel Veditz [:dveditz]]

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.

Comment 18

[Donal Meehan [:dmeehan]]

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

Comment 19

[Tim Huang[:timhuang]]

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

Comment 20

[Tim Huang[:timhuang]]

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

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

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.

Comment 22

[Donal Meehan [:dmeehan]]

(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

Comment 23

[Donal Meehan [:dmeehan]]

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

Approved for 115.0 RC2

Comment 24

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-release/rev/48449fb439a1

Comment 25

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-esr115/rev/bc76b1e5169a

Comment 26

[Ciprian Georgiu, Desktop QA]

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.