1705523 - "source-file" of CSP violation reports use an unsanitized URL

Status: RESOLVED FIXED

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

Description

[Rob Wu [:robwu]]

The "source-file" / sourceFile fields of a CSP violation report are generated by just stripping the ref. This is compliant with the spec:

The result of executing the URL serializer on violation’s source file, with the exclude fragment flag set if the violation’s source file it not null and the empty string otherwise

... but isn't consistent with how the "blocked-url" / blockedURL and "document-url" / documentURL fields are derived: those use StripURIForReporting, which reduces a URL to its scheme if it's not http(s), ws(s) or ftp.

The following are examples of "problematic" cases when the full URL is used instead of just their scheme:

• Long URLs such as data:-URLs
• Opaque URLs such as blob: are reported even when the exact value is not useful.
• Client-specific URLs such as moz-extension://[uuid]/ are reported (bug 1588957).

... so to fix this I propose that we use StripURIForReporting for sourceFile too.

For comparison, Chrome has the following behavior:

• The URL is reduced to a scheme (source code) if the scheme is not http(s), ftp, ws(s) (there is more, but those are Chrome-only schemes). This is consistent with Firefox's existing logic for blockedURL + documentURL.
• The query string is stripped from "source-file" / sourceFile. This goes beyond the current spec, but is a historical artifact:
  • In the past, the post-redirect URL was used, so the query string was stripped in crbug 1074317. The pre-redirect URL was used instead after crbug 932892.

Since the expected behavior is consistent with Chrome, a wpt as a regression test would be nice.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1705523 - Use StripURIForReporting for source-file + tests

Comment 2

[Rob Wu [:robwu]]

I filed a spec issue at https://github.com/w3c/webappsec-csp/issues/489 in an attempt to document the currently implemented behavior in Firefox and Chrome in the specification text.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57d6e95524ef
Use StripURIForReporting for source-file + tests r=ckerschb

Comment 4

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/28580 for changes under testing/web-platform/tests

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/57d6e95524ef