Page MenuHomePhabricator

Bug 1313937 - CSP: Reimplement 'strict-dynamic'. r?freddyb
ClosedPublic

Authored by tschuster on Jun 30 2023, 1:04 PM.
Referenced Files
Unknown Object (File)
Sun, Nov 2, 1:45 PM
Unknown Object (File)
Wed, Oct 15, 12:09 AM
Unknown Object (File)
Mon, Oct 13, 1:16 AM
Unknown Object (File)
Sep 27 2025, 9:31 PM
Unknown Object (File)
Apr 11 2025, 3:22 PM
Unknown Object (File)
Apr 4 2025, 11:16 AM
Unknown Object (File)
Mar 15 2025, 12:17 PM
Unknown Object (File)
Jan 10 2025, 9:17 PM
Subscribers

Diff Detail

Repository
rMOZILLACENTRAL mozilla-central
Branch
default

Event Timeline

evilpie created this revision.
phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".Jun 30 2023, 1:05 PM
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.
tschuster edited the summary of this revision. (Show Details)
tschuster added a reviewer: evilpie.
tschuster removed a reviewer: evilpie.

Code analysis found 5 defects in diff 737815:

  • 1 defect found by clang-format
  • 1 defect found by clang-tidy
  • 3 defects found by clang-format (Mozlint)
WARNING: Found 5 defects (warning level) that can be dismissed.

You can run this analysis locally with:

  • ./mach static-analysis check --outgoing (C/C++)
  • ./mach lint --warnings --outgoing
  • ./mach clang-format -p dom/security/nsCSPParser.cpp

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).


If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 737815.

tschuster updated this revision to Diff 738662.
tschuster retitled this revision from WIP: Bug 1313937 - CSP: Reimplemnt 'strict-dynamic' to WIP: Bug 1313937 - CSP: Reimplement 'strict-dynamic'.
tschuster updated this revision to Diff 741433.
tschuster retitled this revision from WIP: Bug 1313937 - CSP: Reimplement 'strict-dynamic' to Bug 1313937 - CSP: Reimplement 'strict-dynamic'. r?freddyb.
tschuster added a reviewer: freddyb.

Code analysis found 1 defect in diff 741433:

  • 1 defect found by clang-tidy

1 defect unresolved and 4 defects closed compared to the previous diff 738662.

WARNING: Found 1 defect (warning level) that can be dismissed.

You can run this analysis locally with:

  • ./mach static-analysis check --outgoing (C/C++)

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 741433.

freddyb added a project: testing-approved.
freddyb added inline comments.
dom/security/nsCSPContext.cpp
654–663

Can you give an example of a failing tests case?

dom/security/nsCSPUtils.cpp
1131

Nit: Could you break once you have found a single occurrence?

This revision is now accepted and ready to land.Jul 13 2023, 12:56 PM
dom/security/nsCSPContext.cpp
654–663

Good question. We fail /content-security-policy/script-src/script-src-strict_dynamic_meta_tag.html and /content-security-policy/script-src/script-src-strict_dynamic_non_parser_inserted.html with

  • Script injected via appendChild populated via textContent is allowed with strict-dynamic
  • Script injected via appendChild populated via textContent is allowed with strict-dynamic, even if it carries an incorrect nonce.
dom/security/nsCSPUtils.cpp
1131

No, because we still need to collect all hashes in the directive even after find an instance of 'strict-dynamic'.

This revision is now accepted and ready to land.Jul 21 2023, 2:48 PM