Issue #2402 - CSP Violation events should have the correct sample for…

… inline contexts. https://bugzilla.mozilla.org/show_bug.cgi?id=1473587 Add preference to increase max length of CSP report source sample. https://bugzilla.mozilla.org/show_bug.cgi?id=1415352 Return valid columnNumber value in CSP violation events. https://bugzilla.mozilla.org/show_bug.cgi?id=1418246
FranklinDM · Jan 8, 2024 · 3dd8bc0 · 3dd8bc0
1 parent 0fe8ac9
commit 3dd8bc0
Show file tree

Hide file tree

Showing 38 changed files with 277 additions and 106 deletions.
diff --git a/caps/nsScriptSecurityManager.cpp b/caps/nsScriptSecurityManager.cpp
@@ -535,7 +535,8 @@ NS_IMPL_ISUPPORTS(nsScriptSecurityManager,
 ///////////////// Security Checks /////////////////
 
 bool
-nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext *cx)
+nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext *cx,
+                                                              JS::HandleValue aValue)
 {
     MOZ_ASSERT(cx == nsContentUtils::GetCurrentJSContext());
     nsCOMPtr<nsIPrincipal> subjectPrincipal = nsContentUtils::SubjectPrincipal();
@@ -558,12 +559,23 @@ nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext *cx)
     }
 
     if (reportViolation) {
-        nsAutoString fileName;
-        unsigned lineNum = 0;
-        NS_NAMED_LITERAL_STRING(scriptSample, "call to eval() or related function blocked by CSP");
+        JS::Rooted<JSString*> jsString(cx, JS::ToString(cx, aValue));
+        if (NS_WARN_IF(!jsString)) {
+          JS_ClearPendingException(cx);
+          return false;
+        }
+
+        nsAutoJSString scriptSample;
+        if (NS_WARN_IF(!scriptSample.init(cx, jsString))) {
+          JS_ClearPendingException(cx);
+          return false;
+        }
 
         JS::AutoFilename scriptFilename;
-        if (JS::DescribeScriptedCaller(cx, &scriptFilename, &lineNum)) {
+        nsAutoString fileName;
+        unsigned lineNum = 0;
+        unsigned columnNum = 0;
+        if (JS::DescribeScriptedCaller(cx, &scriptFilename, &lineNum, &columnNum)) {
             if (const char *file = scriptFilename.get()) {
                 CopyUTF8toUTF16(nsDependentCString(file), fileName);
             }
@@ -574,6 +586,7 @@ nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext *cx)
                                  fileName,
                                  scriptSample,
                                  lineNum,
+                                 columnNum,
                                  EmptyString(),
                                  EmptyString());
     }

diff --git a/caps/nsScriptSecurityManager.h b/caps/nsScriptSecurityManager.h
@@ -91,7 +91,7 @@ class nsScriptSecurityManager final : public nsIScriptSecurityManager,
 
     // Decides, based on CSP, whether or not eval() and stuff can be executed.
     static bool
-    ContentSecurityPolicyPermitsJSAction(JSContext *cx);
+    ContentSecurityPolicyPermitsJSAction(JSContext *cx, JS::HandleValue aValue);
 
     static bool
     JSPrincipalsSubsume(JSPrincipals *first, JSPrincipals *second);

diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp
@@ -11871,6 +11871,7 @@ nsIDocument::InlineScriptAllowedByCSP()
                                        true,          // aParserCreated
                                        EmptyString(), // FIXME get script sample (bug 1314567)
                                        0,             // aLineNumber
+                                       0,             // aColumnNumber
                                        &allowsInlineScript);
     NS_ENSURE_SUCCESS(rv, true);
   }

diff --git a/dom/base/nsIStyleSheetLinkingElement.h b/dom/base/nsIStyleSheetLinkingElement.h
@@ -101,6 +101,19 @@ class nsIStyleSheetLinkingElement : public nsISupports {
    *         was set
    */
   virtual uint32_t GetLineNumber() = 0;
+
+  // This doesn't entirely belong here since they only make sense for
+  // some types of linking elements, but it's a better place than
+  // anywhere else.
+  virtual void SetColumnNumber(uint32_t aColumnNumber) = 0;
+
+  /**
+   * Get the column number, as previously set by SetColumnNumber.
+   *
+   * @return the column number of this element; or 1 if no column number
+   *         was set
+   */
+  virtual uint32_t GetColumnNumber() = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIStyleSheetLinkingElement,

diff --git a/dom/base/nsStyleLinkElement.cpp b/dom/base/nsStyleLinkElement.cpp
@@ -40,6 +40,7 @@ nsStyleLinkElement::nsStyleLinkElement()
   : mDontLoadStyle(false)
   , mUpdatesEnabled(true)
   , mLineNumber(1)
+  , mColumnNumber(1)
 {
 }
 
@@ -127,6 +128,18 @@ nsStyleLinkElement::GetLineNumber()
   return mLineNumber;
 }
 
+/* virtual */ void
+nsStyleLinkElement::SetColumnNumber(uint32_t aColumnNumber)
+{
+  mColumnNumber = aColumnNumber;
+}
+
+/* virtual */ uint32_t
+nsStyleLinkElement::GetColumnNumber()
+{
+  return mColumnNumber;
+}
+
 /* static */ bool
 nsStyleLinkElement::IsImportEnabled()
 {
@@ -412,8 +425,10 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument,
     if (!nsStyleUtil::CSPAllowsInlineStyle(thisContent,
                                            thisContent->NodePrincipal(),
                                            doc->GetDocumentURI(),
-                                           mLineNumber, text, &rv))
+                                           mLineNumber, mColumnNumber, text,
+                                           &rv)) {
       return rv;
+    }
 
     // Parse the style sheet.
     rv = doc->CSSLoader()->

diff --git a/dom/base/nsStyleLinkElement.h b/dom/base/nsStyleLinkElement.h
@@ -54,6 +54,8 @@ class nsStyleLinkElement : public nsIStyleSheetLinkingElement
   virtual void OverrideBaseURI(nsIURI* aNewBaseURI) override;
   virtual void SetLineNumber(uint32_t aLineNumber) override;
   virtual uint32_t GetLineNumber() override;
+  void SetColumnNumber(uint32_t aColumnNumber) override;
+  uint32_t GetColumnNumber() override;
 
   enum RelValue {
     ePREFETCH =     0x00000001,
@@ -140,6 +142,7 @@ class nsStyleLinkElement : public nsIStyleSheetLinkingElement
   bool mDontLoadStyle;
   bool mUpdatesEnabled;
   uint32_t mLineNumber;
+  uint32_t mColumnNumber;
 };
 
 #endif /* nsStyleLinkElement_h___ */

diff --git a/dom/base/nsStyledElement.cpp b/dom/base/nsStyledElement.cpp
@@ -191,7 +191,7 @@ nsStyledElement::ParseStyleAttribute(const nsAString& aValue,
 
   if (!isNativeAnon &&
       !nsStyleUtil::CSPAllowsInlineStyle(nullptr, NodePrincipal(),
-                                         doc->GetDocumentURI(), 0, aValue,
+                                         doc->GetDocumentURI(), 0, 0, aValue,
                                          nullptr))
     return;
 

diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp
@@ -786,28 +786,22 @@ EventListenerManager::SetEventHandler(nsIAtom* aName,
     rv = doc->NodePrincipal()->GetCsp(getter_AddRefs(csp));
     NS_ENSURE_SUCCESS(rv, rv);
 
-    if (csp) {
-      // let's generate a script sample and pass it as aContent,
-      // it will not match the hash, but allows us to pass
-      // the script sample in aContent.
-      nsAutoString scriptSample, attr, tagName(NS_LITERAL_STRING("UNKNOWN"));
-      aName->ToString(attr);
-      nsCOMPtr<nsIDOMNode> domNode(do_QueryInterface(mTarget));
-      if (domNode) {
-        domNode->GetNodeName(tagName);
-      }
-      // build a "script sample" based on what we know about this element
-      scriptSample.Assign(attr);
-      scriptSample.AppendLiteral(" attribute on ");
-      scriptSample.Append(tagName);
-      scriptSample.AppendLiteral(" element");
+    unsigned lineNum = 0;
+    unsigned columnNum = 0;
 
+    JSContext* cx = nsContentUtils::GetCurrentJSContext();
+    if (cx && !JS::DescribeScriptedCaller(cx, nullptr, &lineNum, &columnNum)) {
+      JS_ClearPendingException(cx);
+    }
+
+    if (csp) {
       bool allowsInlineScript = true;
       rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
                                 EmptyString(), // aNonce
                                 true, // aParserCreated (true because attribute event handler)
-                                scriptSample,
-                                0,             // aLineNumber
+                                aBody,
+                                lineNum,             // aLineNumber
+                                columnNum,           // aColumnNumber
                                 &allowsInlineScript);
       NS_ENSURE_SUCCESS(rv, rv);
 

diff --git a/dom/interfaces/security/nsIContentSecurityPolicy.idl b/dom/interfaces/security/nsIContentSecurityPolicy.idl
@@ -140,6 +140,8 @@ interface nsIContentSecurityPolicy : nsISerializable
    *        (and compare to the hashes listed in the policy)
    * @param aLineNumber The line number of the inline resource
    *        (used for reporting)
+   * @param aColumnNumber The column number of the inline resource
+   *        (used for reporting)
    * @return
    *     Whether or not the effects of the inline style should be allowed
    *     (block the rules if false).
@@ -148,7 +150,8 @@ interface nsIContentSecurityPolicy : nsISerializable
                           in AString aNonce,
                           in boolean aParserCreated,
                           in AString aContent,
-                          in unsigned long aLineNumber);
+                          in unsigned long aLineNumber,
+                          in unsigned long aColumnNumber);
 
   /**
    * whether this policy allows eval and eval-like functions
@@ -188,6 +191,8 @@ interface nsIContentSecurityPolicy : nsISerializable
    *     sample of the violating content (to aid debugging)
    * @param lineNum
    *     source line number of the violation (if available)
+   * @param columnNum
+   *     source column number of the violation (if available)
    * @param aNonce
    *     (optional) If this is a nonce violation, include the nonce so we can
    *     recheck to determine which policies were violated and send the
@@ -202,6 +207,7 @@ interface nsIContentSecurityPolicy : nsISerializable
                            in AString sourceFile,
                            in AString scriptSample,
                            in int32_t lineNum,
+                           in int32_t columnNum,
                            [optional] in AString nonce,
                            [optional] in AString content);
 

diff --git a/dom/jsurl/nsJSProtocolHandler.cpp b/dom/jsurl/nsJSProtocolHandler.cpp
@@ -185,6 +185,7 @@ nsresult nsJSThunk::EvaluateScript(nsIChannel *aChannel,
                                   true,         // aParserCreated
                                   EmptyString(), // aContent
                                   0,             // aLineNumber
+                                  0,             // aColumnNumber
                                   &allowsInlineScript);
 
         //return early if inline scripts are not allowed

diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp
@@ -1473,6 +1473,7 @@ CSPAllowsInlineScript(nsIScriptElement *aElement, nsIDocument *aDocument)
   rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
                             nonce, parserCreated, scriptText,
                             aElement->GetScriptLineNumber(),
+                            aElement->GetScriptColumnNumber(),
                             &allowInlineScript);
   return allowInlineScript;
 }
@@ -2694,10 +2695,11 @@ ScriptLoader::VerifySRI(ScriptLoadRequest* aRequest,
       nsAutoCString violationURISpec;
       mDocument->GetDocumentURI()->GetAsciiSpec(violationURISpec);
       uint32_t lineNo = aRequest->Element() ? aRequest->Element()->GetScriptLineNumber() : 0;
+      uint32_t columnNo = aRequest->Element() ? aRequest->Element()->GetScriptColumnNumber() : 0;
       csp->LogViolationDetails(
         nsIContentSecurityPolicy::VIOLATION_TYPE_REQUIRE_SRI_FOR_SCRIPT,
         NS_ConvertUTF8toUTF16(violationURISpec),
-        EmptyString(), lineNo, EmptyString(), EmptyString());
+        EmptyString(), lineNo, columnNo, EmptyString(), EmptyString());
       rv = NS_ERROR_SRI_CORRUPT;
     }
   }

diff --git a/dom/script/nsIScriptElement.h b/dom/script/nsIScriptElement.h
@@ -30,6 +30,7 @@ class nsIScriptElement : public nsIScriptLoaderObserver {
 
   explicit nsIScriptElement(mozilla::dom::FromParser aFromParser)
     : mLineNumber(1),
+      mColumnNumber(1),
       mAlreadyStarted(false),
       mMalformed(false),
       mDoneAddingChildren(aFromParser == mozilla::dom::NOT_FROM_PARSER ||
@@ -138,6 +139,16 @@ class nsIScriptElement : public nsIScriptLoaderObserver {
     return mLineNumber;
   }
 
+  void SetScriptColumnNumber(uint32_t aColumnNumber)
+  {
+    mColumnNumber = aColumnNumber;
+  }
+
+  uint32_t GetScriptColumnNumber()
+  {
+    return mColumnNumber;
+  }
+
   void SetIsMalformed()
   {
     mMalformed = true;
@@ -281,6 +292,11 @@ class nsIScriptElement : public nsIScriptLoaderObserver {
    */
   uint32_t mLineNumber;
 
+  /**
+   * The start column number of the script.
+   */
+  uint32_t mColumnNumber;
+
   /**
    * The "already started" flag per HTML5.
    */

diff --git a/dom/security/CSPEvalChecker.cpp b/dom/security/CSPEvalChecker.cpp
@@ -46,7 +46,7 @@ CheckInternal(nsIContentSecurityPolicy* aCSP,
   if (reportViolation) {
     aCSP->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL,
                               aFileNameString, aExpression, aLineNum,
-                              EmptyString(), EmptyString());
+                              aColumnNum, EmptyString(), EmptyString());
   }
 
   return NS_OK;