1735856 - (CVE-2022-22745) Securitypolicyviolation leaks cross-origin information into parent for frame-ancestors violations

Status: RESOLVED FIXED

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

Description

[jannis]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:92.0) Gecko/20100101 Firefox/92.0

Steps to reproduce:

Visit https://demo.websec.saarland/static/csp_frame.html
Define a securitypolicyviolation handler and frame an document that sets frame-ancestors that blocks the framing

Actual results:

A securitypolicyviolation event is thrown on the parent page

Expected results:

No securitypolicyviolation should be thrown as this leaks cross-origin information.
See https://bugs.chromium.org/p/chromium/issues/detail?id=1186611&q=frame-ancestors&can=1 for the discussion and fix in chromium.

Comment 1

[Daniel Veditz [:dveditz]]

On the demo page it looks like the whole navigation URL (in this case "https://bad.demo.websec.saarland/echo/?content-security-policy=frame-ancestors%20%27self%27") is leaked to the parent -- although that URL is already known to the parent in this case since there's no redirect involved. Have to make sure that's not confounding the results.

Comment 2

[Daniel Veditz [:dveditz]]

Confirmed by modifying the testcase to load https://bit.ly/3m4Qop7 in the frame. The SecurityPolicyViolationEvent sent to the parent page contains the final post-redirect https://bad.demo.websec.saarland documentURI.

Even if it were the pre-redirect URL, we shouldn't be throwing a violation event from one document's CSP at another document! All the parent gets to learn is that the framed content didn't load through the onerror/onload frame events, not why or which document.

Comment 3

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

The severity field is not set for this bug.
:ckerschb, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Niklas, can you please take a look - would be good to get this one resolved!

Comment 6

[Rob Wu [:robwu]]

I'm taking this to avoid impacting bug 1745566.

Comment 7

[Rob Wu [:robwu]]

Attachment: Bug 1735856 - Don't trigger violation events for frame-ancestors

Comment 8

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

Don't trigger violation events for frame-ancestors r=freddyb
https://hg.mozilla.org/integration/autoland/rev/ccd227e3c8a6d8833b49c7b31e28ff2895e93489
https://hg.mozilla.org/mozilla-central/rev/ccd227e3c8a6

Comment 9

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

The patch landed in nightly and beta is affected.
:robwu, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 10

[Rob Wu [:robwu]]

Comment on attachment 9255902 [details]
Bug 1735856 - Don't trigger violation events for frame-ancestors

Beta/Release Uplift Approval Request

• User impact if declined: Cross-origin infoleak when websites specify the frame-ancestors CSP directive.
• Is this code covered by automated tests?: Yes
• 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): Covered by unit tests (an existing web platform test). A risk of uplifting is that requesting an uplift now could draw attention to this bug.
• String changes made/needed:

Comment 11

[Dianna Smith [:diannaS]]

Comment on attachment 9255902 [details]
Bug 1735856 - Don't trigger violation events for frame-ancestors

Approved for 96.0b9

Comment 12

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e8d3f3044a85

Comment 13

[Dianna Smith [:diannaS]]

I would like to note that this was temporarily backed out during build failure investigation and reintroduced for 96.0b9.
Backout link

Reintroduced in:
https://hg.mozilla.org/releases/mozilla-beta/rev/ed5213c215a7

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Do we need this on ESR also? It grafts cleanly.

Comment 15

[Rob Wu [:robwu]]

Comment on attachment 9255902 [details]
Bug 1735856 - Don't trigger violation events for frame-ancestors

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Easily exploitable cross-origin info leak when websites use the frame-ancestors CSP directive.
• 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): Covered by unit tests (an existing web platform test).

Comment 16

[Frederik Braun [:freddy]]

My advisory draft currently just calls you "Jannis" like your Bugzilla handle, please let me know if any other name or full name would be preferable.

Comment 17

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9255902 [details]
Bug 1735856 - Don't trigger violation events for frame-ancestors

Approved for 91.5esr.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/f5701faab581

Comment 20

[jannis]

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

My advisory draft currently just calls you "Jannis" like your Bugzilla handle, please let me know if any other name or full name would be preferable.

I would prefer my full name: Jannis Rautenstrauch

Comment 21

[Frederik Braun [:freddy]]

Attachment: advisory.txt