Trusted Types violation sample clips incorrectly

https://bugs.webkit.org/show_bug.cgi?id=275392 Reviewed by NOBODY (OOPS!). This patch accurately follows the spec for sample clipping in trusted types. Spec: w3c/webappsec-csp#659 * LayoutTests/imported/w3c/web-platform-tests/content-security-policy/reporting/report-clips-sample.https-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/content-security-policy/reporting/report-clips-sample.https.html: * Source/WebCore/page/csp/ContentSecurityPolicy.cpp: (WebCore::ContentSecurityPolicy::didReceiveHeader): (WebCore::ContentSecurityPolicy::allowMissingTrustedTypesForSinkGroup const): (WebCore::ContentSecurityPolicy::reportViolation const): (WebCore::ContentSecurityPolicy::setUpgradeInsecureRequests): * Source/WebCore/page/csp/ContentSecurityPolicy.h:
lukewarlow · Jul 2, 2024 · 554c67d · 554c67d
1 parent 8913b13
commit 554c67d
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 13 deletions.
diff --git a/...b-platform-tests/content-security-policy/reporting/report-clips-sample.https-expected.txt b/...b-platform-tests/content-security-policy/reporting/report-clips-sample.https-expected.txt
@@ -1,9 +1,7 @@
 
-FAIL Unsafe eval violation sample is clipped to 40 characters. assert_throws_js: function "_ => {
-      eval("evil = '1234567890123456789012345678901234567890';");
-    }" did not throw
+PASS Unsafe eval violation sample is clipped to 40 characters.
 FAIL Function constructor - the other kind of eval - is clipped. assert_throws_js: function "_ => {
       new Function("a", "b", "return '1234567890123456789012345678901234567890';");
     }" did not throw
-FAIL Trusted Types violation sample is clipped to 40 characters excluded the sink name. assert_equals: expected "Element innerHTML|1234567890123456789012345678901234567890" but got "Element innerHTML|1234567890123456789012"
+PASS Trusted Types violation sample is clipped to 40 characters excluded the sink name.
 
diff --git a/...d/w3c/web-platform-tests/content-security-policy/reporting/report-clips-sample.https.html b/...d/w3c/web-platform-tests/content-security-policy/reporting/report-clips-sample.https.html
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html><!-- webkit-test-runner [ jscOptions=--useTrustedTypes=true ] -->
 <html>
 <head>
   <script src="/resources/testharness.js"></script>

diff --git a/Source/WebCore/page/csp/ContentSecurityPolicy.cpp b/Source/WebCore/page/csp/ContentSecurityPolicy.cpp
@@ -270,7 +270,7 @@ void ContentSecurityPolicy::didReceiveHeader(const String& header, ContentSecuri
             begin = buffer.position();
         }
     });
-    
+
 
     if (m_scriptExecutionContext)
         applyPolicyToScriptExecutionContext();
@@ -807,8 +807,8 @@ bool ContentSecurityPolicy::allowMissingTrustedTypesForSinkGroup(const String& s
                 "This requires a "_s, stringContext, " value else it violates the following Content Security Policy directive: \"require-trusted-types-for 'script'\""_s);
 
             TextPosition sourcePosition(OrdinalNumber::beforeFirst(), OrdinalNumber());
-            String sample = makeString(sink, '|', source);
-            reportViolation("require-trusted-types-for"_s, *policy, "trusted-types-sink"_s, consoleMessage, nullString(), sample, sourcePosition, nullptr);
+            String sample = makeString(sink, '|', source.left(40));
+            reportViolation("require-trusted-types-for"_s, *policy, "trusted-types-sink"_s, consoleMessage, nullString(), sample, sourcePosition, nullptr, URL(), nullptr, SamplePreClipped::Yes);
         }
         return isAllowed;
     });
@@ -863,7 +863,7 @@ void ContentSecurityPolicy::reportViolation(const ContentSecurityPolicyDirective
     return reportViolation(violatedDirective.nameForReporting().convertToASCIILowercase(), violatedDirective.directiveList(), blockedURL, consoleMessage, sourceURL, sourceContent, sourcePosition, state, preRedirectURL, element);
 }
 
-void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirective, const ContentSecurityPolicyDirectiveList& violatedDirectiveList, const String& blockedURLString, const String& consoleMessage, const String& sourceURL, const StringView& sourceContent, const TextPosition& sourcePosition, JSC::JSGlobalObject* state, const URL& preRedirectURL, Element* element) const
+void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirective, const ContentSecurityPolicyDirectiveList& violatedDirectiveList, const String& blockedURLString, const String& consoleMessage, const String& sourceURL, const StringView& sourceContent, const TextPosition& sourcePosition, JSC::JSGlobalObject* state, const URL& preRedirectURL, Element* element, SamplePreClipped samplePreClipped) const
 {
     logToConsole(consoleMessage, sourceURL, sourcePosition.m_line, sourcePosition.m_column, state);
 
@@ -886,7 +886,9 @@ void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirec
     info.documentURI = m_documentURL ? m_documentURL.value().strippedForUseAsReferrer().string : blockedURI;
     info.lineNumber = sourcePosition.m_line.oneBasedInt();
     info.columnNumber = sourcePosition.m_column.oneBasedInt();
-    info.sample = violatedDirectiveList.shouldReportSample(effectiveViolatedDirective) ? sourceContent.left(40).toString() : emptyString();
+    info.sample = emptyString();
+    if (violatedDirectiveList.shouldReportSample(effectiveViolatedDirective))
+        info.sample = samplePreClipped == SamplePreClipped::Yes ? sourceContent.toString() : sourceContent.left(40).toString();
 
     if (!m_client) {
         RefPtrAllowingPartiallyDestroyed<Document> document = dynamicDowncast<Document>(m_scriptExecutionContext.get());
@@ -907,7 +909,7 @@ void ContentSecurityPolicy::reportViolation(const String& effectiveViolatedDirec
 
     // FIXME: Is it policy to not use the status code for HTTPS, or is that a bug?
     unsigned short httpStatusCode = m_selfSourceProtocol == "http"_s ? m_httpStatusCode : 0;
-    
+
     // WPT indicate modern Reporting API expect valid status code, regardless of protocol:
     //     content-security-policy/reporting-api/report-to-directive-allowed-in-meta.https.sub.html
     //     content-security-policy/reporting-api/reporting-api-sends-reports-on-violation.https.sub.html
@@ -1160,7 +1162,7 @@ void ContentSecurityPolicy::setUpgradeInsecureRequests(bool upgradeInsecureReque
         upgradeURL.setProtocol("http"_s);
     else if (upgradeURL.protocolIs("wss"_s))
         upgradeURL.setProtocol("ws"_s);
-    
+
     m_insecureNavigationRequestsToUpgrade.add(SecurityOriginData::fromURL(upgradeURL));
 }
 

diff --git a/Source/WebCore/page/csp/ContentSecurityPolicy.h b/Source/WebCore/page/csp/ContentSecurityPolicy.h
@@ -86,6 +86,11 @@ enum class AllowTrustedTypePolicy : uint8_t {
     DisallowedDuplicateName,
 };
 
+enum class SamplePreClipped : bool {
+    Yes,
+    No,
+};
+
 class ContentSecurityPolicy final : public CanMakeThreadSafeCheckedPtr<ContentSecurityPolicy> {
     WTF_MAKE_FAST_ALLOCATED;
     WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR(ContentSecurityPolicy);
@@ -261,7 +266,7 @@ class ContentSecurityPolicy final : public CanMakeThreadSafeCheckedPtr<ContentSe
     void reportViolation(const ContentSecurityPolicyDirective& violatedDirective, const String& blockedURL, const String& consoleMessage, JSC::JSGlobalObject*, StringView sourceContent) const;
     void reportViolation(const String& effectiveViolatedDirective, const ContentSecurityPolicyDirectiveList&, const String& blockedURL, const String& consoleMessage, JSC::JSGlobalObject* = nullptr) const;
     void reportViolation(const ContentSecurityPolicyDirective& violatedDirective, const String& blockedURL, const String& consoleMessage, const String& sourceURL, const StringView& sourceContent, const TextPosition& sourcePosition, const URL& preRedirectURL = URL(), JSC::JSGlobalObject* = nullptr, Element* = nullptr) const;
-    void reportViolation(const String& violatedDirective, const ContentSecurityPolicyDirectiveList& violatedDirectiveList, const String& blockedURL, const String& consoleMessage, const String& sourceURL, const StringView& sourceContent, const TextPosition& sourcePosition, JSC::JSGlobalObject*, const URL& preRedirectURL = URL(), Element* = nullptr) const;
+    void reportViolation(const String& violatedDirective, const ContentSecurityPolicyDirectiveList& violatedDirectiveList, const String& blockedURL, const String& consoleMessage, const String& sourceURL, const StringView& sourceContent, const TextPosition& sourcePosition, JSC::JSGlobalObject*, const URL& preRedirectURL = URL(), Element* = nullptr, SamplePreClipped = SamplePreClipped::No) const;
     void reportBlockedScriptExecutionToInspector(const String& directiveText) const;
 
     // We can never have both a script execution context and a ContentSecurityPolicyClient.