[tor-commits] [tor-browser/tor-browser-78.8.0esr-10.0-1] Bug 1542194: Update blocked-uri in CSP reporting by treating frame naviations as redirects. r=freddyb, dveditz, mixedpuppy, a=pascalc

[sysrqb at torproject.org]

commit b7969c3b8319354766443502066b59f3a1f8e49a
Author: Christoph Kerschbaumer <ckerschb at christophkerschbaumer.com>
Date:   Thu Feb 11 09:09:17 2021 +0000

    Bug 1542194: Update blocked-uri in CSP reporting by treating frame naviations as redirects. r=freddyb,dveditz,mixedpuppy, a=pascalc
    
    Differential Revision: https://phabricator.services.mozilla.com/D103697
---
 dom/security/nsCSPService.cpp                      | 29 ++++++++++++++++++----
 modules/libpref/init/StaticPrefList.yaml           |  5 ++++
 .../test_ext_contentscript_triggeringPrincipal.js  | 11 ++++++++
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/dom/security/nsCSPService.cpp b/dom/security/nsCSPService.cpp
index af50331e2ddd..3fb9abd64ba8 100644
--- a/dom/security/nsCSPService.cpp
+++ b/dom/security/nsCSPService.cpp
@@ -172,12 +172,31 @@ bool subjectToCSP(nsIURI* aURI, nsContentPolicyType aContentType) {
   nsCOMPtr<nsIContentSecurityPolicy> csp = aLoadInfo->GetCsp();
 
   if (csp) {
+    // Generally aOriginalURI denotes the URI before a redirect and hence
+    // will always be a nullptr here. Only exception are frame navigations
+    // which we want to treat as a redirect for the purpose of CSP reporting
+    // and in particular the `blocked-uri` in the CSP report where we want
+    // to report the prePath information.
+    nsCOMPtr<nsIURI> originalURI = nullptr;
+    nsContentPolicyType extType =
+        nsContentUtils::InternalContentPolicyTypeToExternal(contentType);
+    if (extType == nsIContentPolicy::TYPE_SUBDOCUMENT &&
+        !aLoadInfo->GetOriginalFrameSrcLoad() &&
+        mozilla::StaticPrefs::
+            security_csp_truncate_blocked_uri_for_frame_navigations()) {
+      nsAutoCString prePathStr;
+      nsresult rv = aContentLocation->GetPrePath(prePathStr);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = NS_NewURI(getter_AddRefs(originalURI), prePathStr);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
     // obtain the enforcement decision
-    rv = csp->ShouldLoad(contentType, cspEventListener, aContentLocation,
-                         aMimeTypeGuess,
-                         nullptr,  // no redirect, aOriginal URL is null.
-                         aLoadInfo->GetSendCSPViolationEvents(), cspNonce,
-                         parserCreatedScript, aDecision);
+    rv = csp->ShouldLoad(
+        contentType, cspEventListener, aContentLocation, aMimeTypeGuess,
+        originalURI,  // no redirect, unless it's a frame navigation.
+        aLoadInfo->GetSendCSPViolationEvents(), cspNonce,
+        parserCreatedScript, aDecision);
 
     if (NS_CP_REJECTED(*aDecision)) {
       NS_SetRequestBlockingReason(
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
index a0e5d2fd6d4f..eb05986e017c 100644
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -8485,6 +8485,11 @@
   value: 40
   mirror: always
 
+- name: security.csp.truncate_blocked_uri_for_frame_navigations
+  type: bool
+  value: true
+  mirror: always
+
 # TODO: Bug 1324406: Treat 'data:' documents as unique, opaque origins
 # If true, data: URIs will be treated as unique opaque origins, hence will use
 # a NullPrincipal as the security context.
diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
index 993ee071abb0..772d1fd6892e 100644
--- a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
@@ -25,6 +25,12 @@ Services.prefs.setIntPref(
   4096
 );
 
+// Do not trunacate the blocked-uri in CSP reports for frame navigations.
+Services.prefs.setBoolPref(
+  "security.csp.truncate_blocked_uri_for_frame_navigations",
+  false
+);
+
 // ExtensionContent.jsm needs to know when it's running from xpcshell,
 // to use the right timeout for content scripts executed at document_idle.
 ExtensionTestUtils.mockAppInfo();
@@ -831,6 +837,8 @@ function computeBaseURLs(tests, expectedSources, forbiddenSources = {}) {
 
   function* iterSources(test, sources) {
     for (let [source, attrs] of Object.entries(sources)) {
+      // if a source defines attributes (e.g. liveSrc in PAGE_SOURCES etc.) then all
+      // attributes in the source must be matched by the test (see const TEST).
       if (Object.keys(attrs).every(attr => attrs[attr] === test[attr])) {
         yield `${BASE_URL}/${test.src}?source=${source}`;
       }
@@ -1083,6 +1091,9 @@ const TESTS = [
   },
   // TODO: <frame> element, which requires a frameset document.
   {
+    // the blocked-uri for frame-navigations is the pre-path URI. For the
+    // purpose of this test we do not strip the blocked-uri by setting the
+    // preference 'truncate_blocked_uri_for_frame_navigations'
     element: ["iframe", {}],
     src: "iframe.html",
   },