1895770 - about: page CSP Asserts aren't effective

Status: RESOLVED FIXED

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

Description

[Tom Ritter [:tjr] (OOTO until April)]

It was pointed out that the checks against a website running script are not that effective:

  // preferences and downloads allow legacy inline scripts through hash src.
  MOZ_ASSERT(!foundScriptSrc ||
                 StringBeginsWith(aboutSpec, "about:preferences"_ns) ||
                 StringBeginsWith(aboutSpec, "about:settings"_ns) ||
                 StringBeginsWith(aboutSpec, "about:downloads"_ns) ||
                 StringBeginsWith(aboutSpec, "about:fingerprinting"_ns) ||
                 StringBeginsWith(aboutSpec, "about:asrouter"_ns) ||
                 StringBeginsWith(aboutSpec, "about:newtab"_ns) ||
                 StringBeginsWith(aboutSpec, "about:logins"_ns) ||
                 StringBeginsWith(aboutSpec, "about:compat"_ns) ||
                 StringBeginsWith(aboutSpec, "about:welcome"_ns) ||
                 StringBeginsWith(aboutSpec, "about:profiling"_ns) ||
                 StringBeginsWith(aboutSpec, "about:studies"_ns) ||
                 StringBeginsWith(aboutSpec, "about:home"_ns),
             "about: page must not contain a CSP including script-src");

Because a page is required to have default-src; asserting that a page doesn't have script-src (unless it is allowlisted) doesn't really do much, when all the non-allowlisted pages can run script as long as its allowed via default-src.

I think we can just remove this assertion...

Comment 1

[:Gijs (he/him)]

Curious on Freddy's thoughts here and if we can enforce something more meaningful here? E.g. script-src not including unsafe bits nor http/https/data or whatever?

Comment 2

[Tom Ritter [:tjr] (OOTO until April)]

(In reply to :Gijs (he/him) from comment #1)

script-src not including unsafe bits

https://searchfox.org/mozilla-central/rev/dbd654fa899a56a6a2e92f325c4608020e80afae/dom/security/nsContentSecurityUtils.cpp#1410-1441

nor http/https/data or whatever?

https://searchfox.org/mozilla-central/rev/dbd654fa899a56a6a2e92f325c4608020e80afae/dom/security/nsContentSecurityUtils.cpp#1384-1400

We've got those checks at least :)

Comment 3

[:Gijs (he/him)]

(In reply to Tom Ritter [:tjr] from comment #2)

(In reply to :Gijs (he/him) from comment #1)

script-src not including unsafe bits

https://searchfox.org/mozilla-central/rev/dbd654fa899a56a6a2e92f325c4608020e80afae/dom/security/nsContentSecurityUtils.cpp#1410-1441

nor http/https/data or whatever?

https://searchfox.org/mozilla-central/rev/dbd654fa899a56a6a2e92f325c4608020e80afae/dom/security/nsContentSecurityUtils.cpp#1384-1400

We've got those checks at least :)

Oh, phew. Sorry for not reading more carefully.

Might be useful if we could do the web scheme one for default-src and script-src without any exceptions (which I think should work today?). But yeah, otherwise I guess as comment 0 suggests maybe we just drop the foundScriptSrc one.

Comment 4

[Tom Schuster (MoCo)]

It would also be less error prone to actually use the parsed representation of the CSP policy for finding directives, instead of doing string matching on a serialized string.

Comment 5

[Frederik Braun [:freddy]]

+1 to what Tom Schuster said. We should check the parsed CSP rather than string matching.
Something useful to do for our backlog, I could see us exposing a utility function in our CSP code that answers whether a policy satisfies our internal requirements.

Comment 6

[Tom Schuster (MoCo)]

I am going to investigate this, because I want to do something similar for other windows/dialogs.

Comment 7

[Tom Schuster (MoCo)]

Attachment: Bug 1895770 - Improve the verification of the CSP in about: pages. r?tjr!,freddyb!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/459a5df99cd5
Improve the verification of the CSP in about: pages. r=tjr,freddyb,search-reviewers

Comment 9

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/459a5df99cd5