commit 834ae039ea8957f794624ca6d125628e3979569f Author: Christoph Kerschbaumer ckerschb@christophkerschbaumer.com Date: Tue Feb 9 18:42:03 2021 +0000

Bug 1687342: Blocked-URI in CSP reports should be URI before redirects with ref removed r=freddyb,dveditz a=pascalc

Differential Revision: https://phabricator.services.mozilla.com/D103450 --- dom/security/nsCSPContext.cpp | 38 ++++++++-------------- .../test/csp/test_blocked_uri_in_reports.html | 6 ++-- .../frame-src/frame-src-redirect.html.ini | 5 +++ 3 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 3601f5a4f2fc..413044afcc88 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -950,20 +950,18 @@ void nsCSPContext::logToConsole(const char* aName,

/** * Strip URI for reporting according to: - * http://www.w3.org/TR/CSP/#violation-reports + * https://w3c.github.io/webappsec-csp/#security-violation-reports * * @param aURI - * The uri to be stripped for reporting - * @param aSelfURI - * The uri of the protected resource - * which is needed to enforce the SOP. - * @return ASCII serialization of the uri to be reported. + * The URI of the blocked resource. In case of a redirect, this it the + * initial URI the request started out with, not the redirected URI. + * @return The ASCII serialization of the uri to be reported ignoring + * the ref part of the URI. */ -void StripURIForReporting(nsIURI* aURI, nsIURI* aSelfURI, - nsACString& outStrippedURI) { - // 1) If the origin of uri is a globally unique identifier (for example, - // aURI has a scheme of data, blob, or filesystem), then return the - // ASCII serialization of uri’s scheme. +void StripURIForReporting(nsIURI* aURI, nsACString& outStrippedURI) { + // If the origin of aURI is a globally unique identifier (for example, + // aURI has a scheme of data, blob, or filesystem), then + // return the ASCII serialization of uri’s scheme. bool isHttpFtpOrWs = (aURI->SchemeIs("http") || aURI->SchemeIs("https") || aURI->SchemeIs("ftp") || aURI->SchemeIs("ws") || aURI->SchemeIs("wss")); @@ -976,7 +974,7 @@ void StripURIForReporting(nsIURI* aURI, nsIURI* aSelfURI, return; }

- // Return uri, with any fragment component removed. + // Return aURI, with any fragment component removed. aURI->GetSpecIgnoringRef(outStrippedURI); }

@@ -996,7 +994,7 @@ nsresult nsCSPContext::GatherSecurityPolicyViolationEventData(

// document-uri nsAutoCString reportDocumentURI; - StripURIForReporting(mSelfURI, mSelfURI, reportDocumentURI); + StripURIForReporting(mSelfURI, reportDocumentURI); aViolationEventInit.mDocumentURI = NS_ConvertUTF8toUTF16(reportDocumentURI);

// referrer @@ -1004,17 +1002,9 @@ nsresult nsCSPContext::GatherSecurityPolicyViolationEventData(

// blocked-uri if (aBlockedURI) { - // in case of blocking a browsing context (frame) we have to report - // the final URI in case of a redirect. For subresources we report - // the URI before redirects. - nsCOMPtr<nsIURI> uriToReport; - if (aViolatedDirective.EqualsLiteral("frame-src")) { - uriToReport = aBlockedURI; - } else { - uriToReport = aOriginalURI ? aOriginalURI : aBlockedURI; - } nsAutoCString reportBlockedURI; - StripURIForReporting(uriToReport, mSelfURI, reportBlockedURI); + StripURIForReporting(aOriginalURI ? aOriginalURI : aBlockedURI, + reportBlockedURI); aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(reportBlockedURI); } else { aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(aBlockedString); @@ -1914,4 +1904,4 @@ void nsCSPContext::SerializePolicies( }

aPolicies.AppendElements(mIPCPolicies); -} \ No newline at end of file +} diff --git a/dom/security/test/csp/test_blocked_uri_in_reports.html b/dom/security/test/csp/test_blocked_uri_in_reports.html index 0bb986ec3d04..f40d98efc5d3 100644 --- a/dom/security/test/csp/test_blocked_uri_in_reports.html +++ b/dom/security/test/csp/test_blocked_uri_in_reports.html @@ -19,9 +19,9 @@ SimpleTest.waitForExplicitFinish(); * which gets redirected to: * http://test1.example.com/tests/dom/security//test/csp/file_path_matching.js * - * The blocked-uri in the csp-report should be: - * test1.example.com - * instead of: + * The blocked-uri in the csp-report should be the original URI: + * http://example.com/tests/dom/security/test/csp/file_path_matching_redirect_s... + * instead of the redirected URI: * http://test1.example.com/tests/com/security/test/csp/file_path_matching.js * * see also: http://www.w3.org/TR/CSP/#violation-reports diff --git a/testing/web-platform/meta/content-security-policy/frame-src/frame-src-redirect.html.ini b/testing/web-platform/meta/content-security-policy/frame-src/frame-src-redirect.html.ini new file mode 100644 index 000000000000..afc5b552aae4 --- /dev/null +++ b/testing/web-platform/meta/content-security-policy/frame-src/frame-src-redirect.html.ini @@ -0,0 +1,5 @@ +[frame-src-redirect.html] + expected: TIMEOUT + [Redirected iframe src should evaluate both enforced and report-only policies on both original request and when following redirect] + expected: TIMEOUT +