1687342 - (CVE-2021-23968) Firefox leaks full redirect path via CSP violation reports

Status: VERIFIED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Ademar Nowasky Junior]

Attachment: oauth-leak.png

Tested on: Firefox 84.0.2 (64 bits) (Windows)

In Firefox, Content Security Policy (CSP) violation reports triggered by a redirect did not remove path information as required by the CSP specification: https://www.w3.org/TR/CSP/#security-violation-reports

This potentially reveals information about the redirect that would not otherwise be known to the original site. This could be used by a malicious site to obtain sensitive information such as usernames or single-sign-on tokens encoded within the target URLs.

It looks like this is a recurring problem:
https://bugzilla.mozilla.org/show_bug.cgi?id=1208946
https://bugzilla.mozilla.org/show_bug.cgi?id=1069762

Steps to reproduce:
In a scenario where https://example-01.com performs a code 302 redirect to https://example-02.com/?secret=mysecret

1. The page https://attacker.com is served with the header "Content-Security-Policy: frame-src https://example-01.com; report-uri /report"
2. The page https://attacker.com attempts to iframe https://example-01.com
3. The full path of the redirect ( https://example-02.com/?secret=mysecret ) is leaked to the attacker through the violation report.

It's possible to leak the full path of chained redirects by iframing each leaked URL in a new page with a CSP frame-src directive that matches the domain of the leaked URL.

Through this bug it's possible to leak OAuth authorization codes and access tokens (image attached from a real case), thus getting access to the victim's resources.

Comment 1

[:Gijs (he/him)]

Christoph, can you take a look?

Comment 2

[Daniel Veditz [:dveditz]]

We landed tests to catch this in bug 1069762 -- why didn't they catch this? Are the tests getting skipped or removed 6 years later?

Comment 3

[Frederik Braun [:freddy]]

Nice catch! Annoying the test didn't find it.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Frederik Braun [:freddy] from comment #3)

Nice catch! Annoying the test didn't find it.

Yeah, beats me - I am on it.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1687342 - Test blocked-uri in csp-reports after frame redirect.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1687342: Blocked-URI in CSP reports should be URI before redirects with ref removed

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

As discussed, it seems that wpt/content-security-policy/frame-src/frame-src-redirect.html is incorrect. I filed an issue here:
https://github.com/web-platform-tests/wpt/issues/27384

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 9200000 [details]
Bug 1687342: Blocked-URI in CSP reports should be URI before redirects with ref removed

Security Approval Request

• How easily could an exploit be constructed based on the patch?: I think it's fairly straight forward for someone familiar with CSP to identify the problem by looking at the patch.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?:
• If not all supported branches, which bug introduced the flaw?: Bug 1632811
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Not very likely, we have automated tests. On a scale from 1-5 it would be 1. Since it really only affects the CSP reporting mechanism.

Comment 9

[Tom Ritter [:tjr]]

Comment on attachment 9200000 [details]
Bug 1687342: Blocked-URI in CSP reports should be URI before redirects with ref removed

Approved the patch to land and request uplift from Releng.

The test should land after April 1

Comment 10

[Tom Ritter [:tjr]]

Comment on attachment 9199025 [details]
Bug 1687342 - Test blocked-uri in csp-reports after frame redirect.

Test to land after 4/1

Comment 11

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 9200000 [details]
Bug 1687342: Blocked-URI in CSP reports should be URI before redirects with ref removed

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: see https://bugzilla.mozilla.org/show_bug.cgi?id=1687342#c8
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Beta/Release Uplift Approval Request

• User impact if declined: see https://bugzilla.mozilla.org/show_bug.cgi?id=1687342#c8
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• 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):
• String changes made/needed:

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/5f0961ddc5e715f528cab5fe0f59b40eea441418

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/5f0961ddc5e7

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9200000 [details]
Bug 1687342: Blocked-URI in CSP reports should be URI before redirects with ref removed

Approved for uplift in beta and ESR, thanks.

Comment 15

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

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/37dff697a441

Comment 17

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #15)

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Done.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

I attempted to rebase and uplift this and bug 1542194 to ESR78 but encountered difficulties along the way. Individually, both were able to build and pass tests, but with both patches in the stack, I hit wpt failures:
https://treeherder.mozilla.org/logviewer?job_id=329698922&repo=try&lineNumber=2924

No other tests appear to fail in my Try run. Please take a look and provide rebased patches ASAP. Here's a link to the rebased patches if it helps:
https://hg.mozilla.org/try/rev/4e0acbf437d75b22f88b63d606ca9e3e074ac7b2
https://hg.mozilla.org/try/rev/846b900499f1139032b9273243b5c6ec4facb2ae

Comment 19

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)

I attempted to rebase and uplift this and bug 1542194 to ESR78 but encountered difficulties along the way. Individually, both were able to build and pass tests, but with both patches in the stack, I hit wpt failures:
https://treeherder.mozilla.org/logviewer?job_id=329698922&repo=try&lineNumber=2924

No other tests appear to fail in my Try run.

Hey Ryan,
I just checked out the latest ESR-78 code and on my local machine the test navigate-to/unsafe-allow-redirects/blocked-end-of-chain.sub.html already fails even without any patches applied (see log underneath). I then applied patches and tried to evaluate the problem. Honestly I don't think that my patches are causing that problem. In particular, navigate-to uses different entry points into CSP, more precisely csp->GetAllowsNavigateTo().

FWIW, the navigate-to directive is preffed off by default security.csp.enableNavigateTo and only explicitly enabled for the navigate-to tests. How about we just push an *.ini file with the test and explicitly disable the one test?

%--snip---
TEST_START: /content-security-policy/navigate-to/unsafe-allow-redirects/blocked-end-of-chain.sub.html
0:44.82 INFO Closing window 28
0:45.16 pid:1432126 ###!!! [Parent][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
0:45.18 pid:1432126 [Child 1432334, Main Thread] WARNING: Finishing incremental GC in progress during CC: file /home/ckerschb/source/mozilla-esr78/xpcom/base/nsCycleCollector.cpp, line 3295
0:45.74 pid:1432126 ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /tmp/tmph1J689/runtests_leaks_1432036_tab_pid1432407.log
0:46.28 pid:1432126 [Child 1432334, Main Thread] WARNING: Extra shutdown CC: 'i < NORMAL_SHUTDOWN_COLLECTIONS', file /home/ckerschb/source/mozilla-esr78/xpcom/base/nsCycleCollector.cpp, line 3359
0:49.39 WARNING Traceback (most recent call last):
File "/home/ckerschb/source/mozilla-esr78/testing/web-platform/tests/tools/wptserve/wptserve/handlers.py", line 332, in call
rv = self.func(request, response)
File "/home/ckerschb/source/mozilla-esr78/testing/web-platform/tests/content-security-policy/support/report.py", line 50, in main
request.server.stash.take(key=key)
File "/home/ckerschb/source/mozilla-esr78/testing/web-platform/tests/tools/wptserve/wptserve/stash.py", line 180, in take
internal_key = self._wrap_key(key, path)
File "/home/ckerschb/source/mozilla-esr78/testing/web-platform/tests/tools/wptserve/wptserve/stash.py", line 155, in _wrap_key
return (isomorphic_encode(path), uuid.UUID(key).bytes)
File "/usr/lib/python2.7/uuid.py", line 136, in init
raise ValueError('badly formed hexadecimal UUID string')
ValueError: badly formed hexadecimal UUID string

Comment 20

[Ryan VanderMeulen [:RyanVM]]

That's not a real failure, just logspam. The real failure line is:

TEST-UNEXPECTED-FAIL | /content-security-policy/reporting-api/reporting-api-works-on-frame-src.https.sub.html | Event is fired - assert_equals: expected "https://web-platform.test:8443/content-security-policy/support/fail.html" but got "https://web-platform.test:8443/"

Comment 21

[Timea Cernea [:tbabos][inactive]]

Hi Ademar,
Could you provide by any chance a test page that could be used to verify this manually? Unfortunately, the ones used in 1208946 and 1069762 no longer work now.

Comment 22

[Christoph Kerschbaumer [:ckerschb]]

Attachment: esr78_make_inital_frame_src_load.patch

Hey Ryan and Matt (as the DocumentChannel expert).

In order to successfully uplift Bug 1542194 to esr78 we also need to make sure that loadInfo->GetOriginalFrameSrcLoad() works correctly on esr78. Currently that is not the case because the only place where we set loadInfo->SetOriginalFrameSrcLoad() is within CreateAndConfigureRealChannelForLoadState which is called when running mozilla-central but not called on esr78 when running e.g. reporting-api-works-on-frame-src.https.sub.html.

It's a hard and time consuming task to figure out why that function is not called, but what I can tell for sure is that the problem is manifold, related to the DocumentChannel and that the easiest, least risky and probably simplest solution is what I provided in the attached patch.

In particular it's a mix of:

• Correctly propagating that originalFrameSrcLoad-bit in the loadinfo from Bug 1646573 which landed in FF83.
• Additionally there is definitely no harm in setting the originalFrameSrcLoad-bit in nsDocShell in case of an iframe load. We do basically the equivalent for SetHasValidUserGestureActivation right above.

Instead of going down the rabbit hole and figuring out technical differences between current mozilla-central and esr78, I really think it's easiest and most likely the least risky patch to uplift if we just explicitly set the originalFrameSrcLoad-bit on the loadinfo and make sure the bit is correctly propagated.

Any objections?

Comment 23

[Ademar Nowasky Junior]

(In reply to Timea Cernea [:tbabos] from comment #21)

Hi Ademar,
Could you provide by any chance a test page that could be used to verify this manually? Unfortunately, the ones used in 1208946 and 1069762 no longer work now.

Hi Timea,

I just created a scenario where this bug can be tested: https://nowasky2.herokuapp.com/

It will show an alert for these 3 cases:

1. The CSP violation report contains the full redirect path
2. The CSP violation report contains only the redirect domain
3. The CSP violation report does not leak any redirect data

Comment 24

[Timea Cernea [:tbabos][inactive]]

Thank you very much for the testcase!

Reproduced the issue on the affected Firefox Release 85.0.2:

• CSP violation report leaked the full redirect path - blocked-uri: https://www.example.com/?secret=mysecret

Verified-fixed on latest Firefox Nightly 87.0a1 (2021-02-15) (64-bit) and Firefox Beta RC 86.0 on Windows 10 x64 and MacOS 10.15:

• CSP violation report did not leak any redirect data - blocked-uri: https://bit.ly/3jTNHnE

Waiting for the fix to land on ESR for further verification.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #22)

Any objections?

I'll defer to Matt's risk analysis on this, but offhand this seems like a reasonable plan to me.

Comment 26

[Matt Woodrow (:mattwoodrow)]

Yeah, that seems like a simple and reasonable thing to uplift to fix this :)

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9203250 [details] [diff] [review]
esr78_make_inital_frame_src_load.patch

Thanks for figuring this out! Approved to ride along with the other ESR patch.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/93aa4e03d7a1

Comment 29

[Timea Cernea [:tbabos][inactive]]

Verified-fixed on the latest ESR 78.8.0esr (64-bit)(20210217034806) version on Windows 10 x64 and MacOS 10.15.

Comment 30

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 31

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

Test blocked-uri in csp-reports after frame redirect. r=freddyb,dveditz
https://hg.mozilla.org/integration/autoland/rev/c80b0185fdc476236b31eda784a17c66a3e4d24f
https://hg.mozilla.org/mozilla-central/rev/c80b0185fdc4