CSP_MANIFEST warning description is inaccurate, especially in MV3

[Rob--W]

The CSP_MANIFEST warning is emitted when a CSP is insecure. Its message is defined to be "content_security_policy allows remote code execution in manifest.json" (or "content_security_policy.extension_pages allows remote code execution in manifest.json" in MV3).

The description is "A custom content_security_policy needs additional review".

In Manifest V2, the message was sometimes accurate. In Manifest V3, the message is misleading.

In Manifest V2, a custom CSP can relax the default strict CSP. E.g. https: or 'unsafe-eval' can be added to relax the CSP.
In Manifest V3, a custom CSP cannot relax the default CSP to enable remote code.

There are four relevant possibilities given a CSP string:

• It is secure.
  • e.g. 'script-src 'self'; style-src https:
• It contains an unrecognized or unsupported value, but is still secure.
  • e.g. 'script-src 'self' junk-here
  • The current implementation takes an allowlist approach: only known values are accepted as secure, unknown values are treated as insecure. The advantage of this is that if a new "remote" source is introduced, then the validation cannot be bypassed. The disadvantage is false warnings, such as Invalid CSP not detected #2634.
  • This case is often a developer mistake/typo (e.g. Invalid CSP not detected #2634), or an attempt to use a new CSP feature that wasn't known at the time of the development of a CSP parser (whether in the linter or in Firefox).
• It contains an insecure source, but it is rejected by the browser.
  • e.g. img-src 'none' (from #4578) or 'script-src 'self' 'nonce-123'
  • Firefox does its own validation and rejects invalid sources. Sometimes it rejects the whole CSP (e.g. CSP nonces, 'strict-dynamic', 'unsafe-inline' and more - e.g. #4518), sometimes it's enforced without rejecting the whole CSP (e.g. CSP hashes - https://bugzilla.mozilla.org/show_bug.cgi?id=1789759).
  • There is value in continuing to warn about this, for two reasons:
    1. Extension developers expecting a relaxed policy can then learn that it does not work.
    2. The invalidation of a CSP can result in a weaker CSP being applied than intended (test case in #4578). This issue would disappear (at least in MV3) when https://bugzilla.mozilla.org/show_bug.cgi?id=1799779 is fixed.
• It contains an insecure source, and accepted by the browser.
  • e.g. script-src 'unsafe-eval' (only in MV2)
  • In Manifest V3, this situation does not occur. Everything here is part of "rejected by the browser".
  • This is only possible with manifest V2. E.g. script-src 'unsafe-eval' 'self' is accepted.
    • The current CSP_MANIFEST and CSP_MANIFEST_UNSAFE_EVAL messages target this case.

In short, I propose to change the MANIFEST_CSP message (and for MV3, also CSP_MANIFEST_UNSAFE_EVAL) to the following:

• "content_security_policy contains an invalid or unsupported value." + "A custom content_security_policy should only contain secure and supported sources"

If there is a desire to keep warning about the dangerous last case, MV2-only, we can also keep the current message:

• "content_security_policy allows remote code execution in manifest.json" + "A custom content_security_policy needs additional review"

┆Issue is synchronized with this Jira Task

[stale]

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.