1800890 - (CVE-2023-23602) connect-src CSP header does not apply to WebWorker initiated WebSocket connections

Status: VERIFIED FIXED

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

Description

[Dave Vandyke]

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

Steps to reproduce:

1. Navigate to https://testpages.kzar.co.uk/csp/?csp=connect-src+%27none%27

Actual results:

"WebSocket (third-party via WebWorker via inline script)" is listed as Loaded.

Expected results:

"WebSocket (third-party via WebWorker via inline script)" is listed as Failed. (Which happens with Chrome 107.0.5304.87)

Comment 1

[Dave Vandyke]

I've updated the test page again, so now it's also checking that a message is successfully echoed back from the WebSocket server too.

Comment 2

[Tom Schuster (MoCo)]

So I think there are probably two issues here in Firefox:

1. The smaller/known issues is that Blob URLs generally don't inherit CSPs (bug 1626566). That means even if it worked, we would not be applying the CSP of the loading page in the Worker.
2. We weren't correctly checking the CSP when creating a WebSocket in a Worker even when that Worker had a CSP (i.e. provided via the CSP header).

Comment 3

[Tom Schuster (MoCo)]

Attachment: Bug 1800890 r?asuth!

Comment 4

[Dave Vandyke]

Thanks for looking into this Tom.

1. The smaller/known issues is that Blob URLs generally don't inherit CSPs...

It's maybe worth point out that my test page does also have a test for opening a WebSocket via a blob URI iframe, which seems to be successfully blocked by the CSP.

Comment 5

[Tom Schuster (MoCo)]

Seems like you are right. It also seems like we inherit the CSP from the creating page when using a blob URL for the Worker constructor. I am actually not sure anymore, which behavior is actually correct. As I expected we still seem to respect an explicit Content-Security-Policy header when the Worker is delivered via HTTP.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

My understanding is:

• A worker loaded via a Blob URL should inherit the CSP of the document that minted the Blob URL (which should pedantically be the parent of the dedicated worker itself but there may be some edge cases in there). Spec-wise:
  • Scheme fetch for "blob" is explicitly only providing content-length and content-type headers.
  • Create a policy container from a fetch response explicitly clones a policy container for blobs.
• A worker loaded via HTTP(S) should use the CSP served with the top-level script load of the worker. (The above links are relevant for this too.) As discussed in https://github.com/w3c/webappsec-csp/issues/336 we do think there is some divergence between what the tests say about this and what the correct thing is, but it's my understanding that we have some level of consensus that this is correct but spec changes may be needed.
  • (Note that the initial top-level fetch of the worker may be impacted by the CSP of the window creating the Worker instance, however, but I am not 100% on that at this moment, but we're talking about the websocket creation in the worker which is only constrained by what the worker CSP says.)

Comment 7

[Dave Vandyke]

I've updated the test page again Tom. For the Blob iframe, I'm now taking care to put all the WebSocket opening code in the Blob URI itself (like we are doing for the WebWorker test). I'm still seeing the same results though!

Comment 8

[Tom Schuster (MoCo)]

Attachment: Bug 1800890 - Test r?asuth

Depends on D162827

Comment 9

[Tom Schuster (MoCo)]

https://hg.mozilla.org/integration/autoland/rev/dc87f090ce7a879699f5226ea4f014e7e29a6326

Comment 10

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

r=asuth
https://hg.mozilla.org/mozilla-central/rev/dc87f090ce7a

Comment 11

[Ryan VanderMeulen [:RyanVM]]

If we want to uplift this to ESR102 eventually, a rebased patch is going to be needed.

Comment 12

[Anca Soncutean, Desktop QA]

Reproduced with Fx 107.0 on Ubuntu 22.
Verified fixed on Fx 109.0a1 (2022-12-06) on Windows 10, macOS 12 and Ubuntu 22.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Please attach a rebased patch for ESR102 uplift and nominate for approval when you get a chance.

Comment 14

[Tom Schuster (MoCo)]

Attachment: Bug 1800890 - ESR r?asuth!

Comment 15

[Frederik Braun [:freddy]]

We're getting release tracking alerts for ESR. Andrew, can you review this in time? If not, we'll find another reviewer.

Comment 16

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

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 17

[Tom Schuster (MoCo)]

Comment on attachment 9309345 [details]
Bug 1800890 - ESR r?asuth!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Uplift was requested
• User impact if declined:
• Fix Landed on Version: 109
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9309345 [details]
Bug 1800890 - ESR r?asuth!

Approved for 102.7esr, thanks for rebasing.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/b9be0509188d

Comment 20

[Anca Soncutean, Desktop QA]

Verified fixed on Fx 102.7esr on Windows 10, macOS 13 and Ubuntu 22.

Comment 21

[Wladimir Palant]

Shouldn’t this report be eligible for a bug bounty?

Comment 22

[Frederik Braun [:freddy]]

The reporter seems to not have used our Bug bounty form, so we never had the sec-bounty? flag set, which is what we use for internal triage. We often times get people report bugs without any wish to be credited or rewarded, so there's no process to go through bugs without the flag.

Either way, we can easily change that: Dave do you want us to take a look whether this qualifies for a bug bounty?

The final decision is at the discretion of the bug bounty committee. Our docs and FAQ have more info https://www.mozilla.org/en-US/security/client-bug-bounty/

Comment 23

[Dave Vandyke]

The reporter seems to not have used our Bug bounty form...

Sorry, I didn't realise I was supposed to do that. Is the process to file the issue first, then fill out the form after the issue is accepted?

Dave do you want us to take a look whether this qualifies for a bug bounty?

Yes please, that would be great!

Comment 24

[Frederik Braun [:freddy]]

(In reply to Dave Vandyke from comment #23)

The reporter seems to not have used our Bug bounty form...
Sorry, I didn't realise I was supposed to do that. Is the process to file the issue first, then fill out the form after the issue is accepted?

This is just the typical "file a new bug" form with "hidden security bug" and "please consider me for a bounty" preset. I just set the last flag directly. Not a problem.

Dave do you want us to take a look whether this qualifies for a bug bounty?

Yes please, that would be great!

The bug bounty team is going to look at this in one of the next meetings. It may take a couple of weeks until you hear more.

Comment 25

[June Wilde (she/her) [:jewilde]]

Attachment: advisory.txt

Comment 26

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

2 months ago, Tom Schuster (MoCo) placed a reminder on the bug using the whiteboard tag [reminder-test 2023-02-01] .

tschuster, please refer to the original comment to better understand the reason for the reminder.

Comment 27

[Tom Schuster (MoCo)]

Bug 1800890 - Test r=asuth
https://hg.mozilla.org/integration/autoland/rev/fd312e73e7b7b1619910bdf8f39175ff7b6069bc

Comment 28

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

https://hg.mozilla.org/mozilla-central/rev/fd312e73e7b7

Comment 29

[Tom Schuster (MoCo)]

It seems like we might not automatically sync WPT added in a secure bug?