Bug 1418243 - Fix SecurityPolicyViolationEvent.violatedDirective. draft
authorChung-Sheng Fu <cfu@mozilla.com>
Thu, 21 Dec 2017 11:05:23 +0800
changeset 720743 1b592fb3b0333412457002099506f280805a4426
parent 720581 8e33bdf820dcc2ecd6f326eb83c39fc5e4769dcf
child 720744 dc9aebda134fe1bc007073b454f28c8e812e8e76
push id95616
push userbmo:cfu@mozilla.com
push dateTue, 16 Jan 2018 05:18:37 +0000
bugs1418243
milestone59.0a1
Bug 1418243 - Fix SecurityPolicyViolationEvent.violatedDirective. MozReview-Commit-ID: 8DQ7CI5exUL
dom/security/nsCSPContext.cpp
dom/security/nsCSPUtils.cpp
dom/security/nsCSPUtils.h
--- a/dom/security/nsCSPContext.cpp
+++ b/dom/security/nsCSPContext.cpp
@@ -1,14 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include <string>
+#include <unordered_set>
+
 #include "nsCOMPtr.h"
 #include "nsContentPolicyUtils.h"
 #include "nsContentUtils.h"
 #include "nsCSPContext.h"
 #include "nsCSPParser.h"
 #include "nsCSPService.h"
 #include "nsError.h"
 #include "nsIAsyncVerifyRedirectCallback.h"
@@ -59,16 +62,41 @@ GetCspContextLog()
   return gCspContextPRLog;
 }
 
 #define CSPCONTEXTLOG(args) MOZ_LOG(GetCspContextLog(), mozilla::LogLevel::Debug, args)
 #define CSPCONTEXTLOGENABLED() MOZ_LOG_TEST(GetCspContextLog(), mozilla::LogLevel::Debug)
 
 static const uint32_t CSP_CACHE_URI_CUTOFF_SIZE = 512;
 
+#ifdef DEBUG
+/**
+ * This function is only used for MOZ_ASSERT in
+ * nsCSPContext::GatherSecurityPolicyViolationEventData, so only compile it
+ * in debug builds in order to avoid build error caused by -Wunused-function
+ * in release builds.
+ */
+static bool
+ValidateDirectiveName(const nsAString& aDirective)
+{
+  static const auto directives = [] () {
+    std::unordered_set<std::string> directives;
+    constexpr size_t dirLen = sizeof(CSPStrDirectives) / sizeof(CSPStrDirectives[0]);
+    for (size_t i = 0; i < dirLen; ++i) {
+      directives.insert(CSPStrDirectives[i]);
+    }
+    return directives;
+  } ();
+
+  nsAutoString directive(aDirective);
+  auto itr = directives.find(NS_ConvertUTF16toUTF8(directive).get());
+  return itr != directives.end();
+}
+#endif // DEBUG
+
 /**
  * Creates a key for use in the ShouldLoad cache.
  * Looks like: <uri>!<nsIContentPolicy::LOAD_TYPE>
  */
 nsresult
 CreateCacheKey_Internal(nsIURI* aContentLocation,
                         nsContentPolicyType aContentType,
                         nsACString& outCacheKey)
@@ -864,16 +892,18 @@ nsCSPContext::GatherSecurityPolicyViolat
   uint32_t aViolatedPolicyIndex,
   nsAString& aSourceFile,
   nsAString& aScriptSample,
   uint32_t aLineNum,
   mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit)
 {
   NS_ENSURE_ARG_MAX(aViolatedPolicyIndex, mPolicies.Length() - 1);
 
+  MOZ_ASSERT(ValidateDirectiveName(aViolatedDirective), "Invalid directive name");
+
   nsresult rv;
 
   // document-uri
   nsAutoCString reportDocumentURI;
   StripURIForReporting(mSelfURI, mSelfURI, reportDocumentURI);
   aViolationEventInit.mDocumentURI = NS_ConvertUTF8toUTF16(reportDocumentURI);
 
   // referrer
@@ -895,21 +925,24 @@ nsCSPContext::GatherSecurityPolicyViolat
     if (reportBlockedURI.IsEmpty()) {
       // this can happen for frame-ancestors violation where the violating
       // ancestor is cross-origin.
       NS_WARNING("No blocked URI (null aBlockedContentSource) for CSP violation report.");
     }
     aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(reportBlockedURI);
   }
 
-  // violated-directive
-  aViolationEventInit.mViolatedDirective = aViolatedDirective;
+  // effective-directive
+  // The name of the policy directive that was violated.
+  aViolationEventInit.mEffectiveDirective = aViolatedDirective;
 
-  // effective-directive
-  aViolationEventInit.mEffectiveDirective = aViolatedDirective;
+  // violated-directive
+  // In CSP2, the policy directive that was violated, as it appears in the policy.
+  // In CSP3, the same as effective-directive.
+  aViolationEventInit.mViolatedDirective = aViolatedDirective;
 
   // original-policy
   nsAutoString originalPolicy;
   rv = this->GetPolicyString(aViolatedPolicyIndex, originalPolicy);
   NS_ENSURE_SUCCESS(rv, rv);
   aViolationEventInit.mOriginalPolicy = originalPolicy;
 
   // source-file
@@ -1211,28 +1244,31 @@ class CSPReportSenderRunnable final : pu
         mObserverSubject = do_QueryInterface(supportscstr);
       }
     }
 
     NS_IMETHOD Run() override
     {
       MOZ_ASSERT(NS_IsMainThread());
 
+      nsresult rv;
+
       // 0) prepare violation data
       mozilla::dom::SecurityPolicyViolationEventInit init;
-      mCSPContext->GatherSecurityPolicyViolationEventData(
+      rv = mCSPContext->GatherSecurityPolicyViolationEventData(
         mBlockedContentSource, mOriginalURI,
         mViolatedDirective, mViolatedPolicyIndex,
         mSourceFile, mScriptSample, mLineNum,
         init);
+      NS_ENSURE_SUCCESS(rv, rv);
 
       // 1) notify observers
       nsCOMPtr<nsIObserverService> observerService = mozilla::services::GetObserverService();
       NS_ASSERTION(observerService, "needs observer service");
-      nsresult rv = observerService->NotifyObservers(mObserverSubject,
+      rv = observerService->NotifyObservers(mObserverSubject,
                                                      CSP_VIOLATION_TOPIC,
                                                      mViolatedDirective.get());
       NS_ENSURE_SUCCESS(rv, rv);
 
       // 2) send reports for the policy that was violated
       mCSPContext->SendReports(init, mViolatedPolicyIndex);
 
       // 3) log to console (one per policy violation)
--- a/dom/security/nsCSPUtils.cpp
+++ b/dom/security/nsCSPUtils.cpp
@@ -1238,16 +1238,22 @@ nsCSPDirective::visitSrcs(nsCSPSrcVisito
   return true;
 }
 
 bool nsCSPDirective::equals(CSPDirective aDirective) const
 {
   return (mDirective == aDirective);
 }
 
+void
+nsCSPDirective::getDirName(nsAString& outStr) const
+{
+  outStr.AppendASCII(CSP_CSPDirectiveToString(mDirective));
+}
+
 /* =============== nsCSPChildSrcDirective ============= */
 
 nsCSPChildSrcDirective::nsCSPChildSrcDirective(CSPDirective aDirective)
   : nsCSPDirective(aDirective)
   , mRestrictFrames(false)
   , mRestrictWorkers(false)
 {
 }
@@ -1323,16 +1329,23 @@ nsBlockAllMixedContentDirective::~nsBloc
 
 void
 nsBlockAllMixedContentDirective::toString(nsAString& outStr) const
 {
   outStr.AppendASCII(CSP_CSPDirectiveToString(
     nsIContentSecurityPolicy::BLOCK_ALL_MIXED_CONTENT));
 }
 
+void
+nsBlockAllMixedContentDirective::getDirName(nsAString& outStr) const
+{
+  outStr.AppendASCII(CSP_CSPDirectiveToString(
+    nsIContentSecurityPolicy::BLOCK_ALL_MIXED_CONTENT));
+}
+
 /* =============== nsUpgradeInsecureDirective ============= */
 
 nsUpgradeInsecureDirective::nsUpgradeInsecureDirective(CSPDirective aDirective)
 : nsCSPDirective(aDirective)
 {
 }
 
 nsUpgradeInsecureDirective::~nsUpgradeInsecureDirective()
@@ -1341,16 +1354,23 @@ nsUpgradeInsecureDirective::~nsUpgradeIn
 
 void
 nsUpgradeInsecureDirective::toString(nsAString& outStr) const
 {
   outStr.AppendASCII(CSP_CSPDirectiveToString(
     nsIContentSecurityPolicy::UPGRADE_IF_INSECURE_DIRECTIVE));
 }
 
+void
+nsUpgradeInsecureDirective::getDirName(nsAString& outStr) const
+{
+  outStr.AppendASCII(CSP_CSPDirectiveToString(
+    nsIContentSecurityPolicy::UPGRADE_IF_INSECURE_DIRECTIVE));
+}
+
 /* ===== nsRequireSRIForDirective ========================= */
 
 nsRequireSRIForDirective::nsRequireSRIForDirective(CSPDirective aDirective)
 : nsCSPDirective(aDirective)
 {
 }
 
 nsRequireSRIForDirective::~nsRequireSRIForDirective()
@@ -1392,16 +1412,23 @@ nsRequireSRIForDirective::restrictsConte
 bool
 nsRequireSRIForDirective::allows(enum CSPKeyword aKeyword, const nsAString& aHashOrNonce,
                                  bool aParserCreated) const
 {
   // can only disallow CSP_REQUIRE_SRI_FOR.
   return (aKeyword != CSP_REQUIRE_SRI_FOR);
 }
 
+void
+nsRequireSRIForDirective::getDirName(nsAString& outStr) const
+{
+  outStr.AppendASCII(CSP_CSPDirectiveToString(
+    nsIContentSecurityPolicy::REQUIRE_SRI_FOR));
+}
+
 /* ===== nsCSPPolicy ========================= */
 
 nsCSPPolicy::nsCSPPolicy()
   : mUpgradeInsecDir(nullptr)
   , mReportOnly(false)
 {
   CSPUTILSLOG(("nsCSPPolicy::nsCSPPolicy"));
 }
@@ -1445,32 +1472,32 @@ nsCSPPolicy::permits(CSPDirective aDir,
   nsCSPDirective* defaultDir = nullptr;
 
   // Try to find a relevant directive
   // These directive arrays are short (1-5 elements), not worth using a hashtable.
   for (uint32_t i = 0; i < mDirectives.Length(); i++) {
     if (mDirectives[i]->equals(aDir)) {
       if (!mDirectives[i]->permits(aUri, aNonce, aWasRedirected, mReportOnly,
                                    mUpgradeInsecDir, aParserCreated)) {
-        mDirectives[i]->toString(outViolatedDirective);
+        mDirectives[i]->getDirName(outViolatedDirective);
         return false;
       }
       return true;
     }
     if (mDirectives[i]->isDefaultDirective()) {
       defaultDir = mDirectives[i];
     }
   }
 
   // If the above loop runs through, we haven't found a matching directive.
   // Avoid relooping, just store the result of default-src while looping.
   if (!aSpecific && defaultDir) {
     if (!defaultDir->permits(aUri, aNonce, aWasRedirected, mReportOnly,
                              mUpgradeInsecDir, aParserCreated)) {
-      defaultDir->toString(outViolatedDirective);
+      defaultDir->getDirName(outViolatedDirective);
       return false;
     }
     return true;
   }
 
   // Nothing restricts this, so we're allowing the load
   // See bug 764937
   return true;
@@ -1587,27 +1614,27 @@ nsCSPPolicy::hasDirective(CSPDirective a
  */
 void
 nsCSPPolicy::getDirectiveStringForContentType(nsContentPolicyType aContentType,
                                               nsAString& outDirective) const
 {
   nsCSPDirective* defaultDir = nullptr;
   for (uint32_t i = 0; i < mDirectives.Length(); i++) {
     if (mDirectives[i]->restrictsContentType(aContentType)) {
-      mDirectives[i]->toString(outDirective);
+      mDirectives[i]->getDirName(outDirective);
       return;
     }
     if (mDirectives[i]->isDefaultDirective()) {
       defaultDir = mDirectives[i];
     }
   }
   // if we haven't found a matching directive yet,
   // the contentType must be restricted by the default directive
   if (defaultDir) {
-    defaultDir->toString(outDirective);
+    defaultDir->getDirName(outDirective);
     return;
   }
   NS_ASSERTION(false, "Can not query directive string for contentType!");
   outDirective.AppendASCII("couldNotQueryViolatedDirective");
 }
 
 void
 nsCSPPolicy::getDirectiveAsString(CSPDirective aDir, nsAString& outDirective) const
--- a/dom/security/nsCSPUtils.h
+++ b/dom/security/nsCSPUtils.h
@@ -466,16 +466,18 @@ class nsCSPDirective {
      { return mDirective == nsIContentSecurityPolicy::DEFAULT_SRC_DIRECTIVE; }
 
     virtual bool equals(CSPDirective aDirective) const;
 
     void getReportURIs(nsTArray<nsString> &outReportURIs) const;
 
     bool visitSrcs(nsCSPSrcVisitor* aVisitor) const;
 
+    virtual void getDirName(nsAString& outStr) const;
+
   protected:
     CSPDirective            mDirective;
     nsTArray<nsCSPBaseSrc*> mSrcs;
 };
 
 /* =============== nsCSPChildSrcDirective ============= */
 
 /*
@@ -544,16 +546,18 @@ class nsBlockAllMixedContentDirective : 
     bool allows(enum CSPKeyword aKeyword, const nsAString& aHashOrNonce,
                 bool aParserCreated) const override
       { return false; }
 
     void toString(nsAString& outStr) const override;
 
     void addSrcs(const nsTArray<nsCSPBaseSrc*>& aSrcs) override
       {  MOZ_ASSERT(false, "block-all-mixed-content does not hold any srcs"); }
+
+    void getDirName(nsAString& outStr) const override;
 };
 
 /* =============== nsUpgradeInsecureDirective === */
 
 /*
  * Upgrading insecure requests includes the following actors:
  * (1) CSP:
  *     The CSP implementation whitelists the http-request
@@ -597,16 +601,18 @@ class nsUpgradeInsecureDirective : publi
     bool allows(enum CSPKeyword aKeyword, const nsAString& aHashOrNonce,
                 bool aParserCreated) const override
       { return false; }
 
     void toString(nsAString& outStr) const override;
 
     void addSrcs(const nsTArray<nsCSPBaseSrc*>& aSrcs) override
       {  MOZ_ASSERT(false, "upgrade-insecure-requests does not hold any srcs"); }
+
+    void getDirName(nsAString& outStr) const override;
 };
 
 /* ===== nsRequireSRIForDirective ========================= */
 
 class nsRequireSRIForDirective : public nsCSPDirective {
   public:
     explicit nsRequireSRIForDirective(CSPDirective aDirective);
     ~nsRequireSRIForDirective();
@@ -614,16 +620,17 @@ class nsRequireSRIForDirective : public 
     void toString(nsAString& outStr) const override;
 
     void addType(nsContentPolicyType aType)
       { mTypes.AppendElement(aType); }
     bool hasType(nsContentPolicyType aType) const;
     bool restrictsContentType(nsContentPolicyType aType) const override;
     bool allows(enum CSPKeyword aKeyword, const nsAString& aHashOrNonce,
                 bool aParserCreated) const override;
+    void getDirName(nsAString& outStr) const override;
 
   private:
     nsTArray<nsContentPolicyType> mTypes;
 };
 
 /* =============== nsCSPPolicy ================== */
 
 class nsCSPPolicy {