Bug 1418243 - Fix SecurityPolicyViolationEvent.violatedDirective. r=ckerschb
authorChung-Sheng Fu <cfu@mozilla.com>
Tue, 16 Jan 2018 22:59:00 +0200
changeset 453954 25fd1d8d42b47c72493fdabcdb27c5a1708a01ad
parent 453953 41bb3e170bf6a10db95c2a92e8d639a8e6347256
child 453955 6b40a9ee5e9ba361bdd249b1e81b00ab329fdb08
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckerschb
bugs1418243
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1418243 - Fix SecurityPolicyViolationEvent.violatedDirective. r=ckerschb 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,39 @@ 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 verification purposes within
+ * GatherSecurityPolicyViolationEventData.
+ */
+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 +890,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 +923,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 +1242,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
@@ -1241,16 +1241,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)
 {
 }
@@ -1326,16 +1332,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()
@@ -1344,16 +1357,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()
@@ -1395,16 +1415,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"));
 }
@@ -1448,32 +1475,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;
@@ -1590,27 +1617,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 {