From ce931385689aebd4aaadfe634574622464d63e9b Mon Sep 17 00:00:00 2001
From: Bobby Holley <bobbyholley@gmail.com>
Date: Wed, 19 Aug 2015 11:13:15 -0700
Subject: [PATCH] Bug 1196371 - Add a runtime assertion against illegal string
 characters in OriginAttributes suffix creation. r=janv,r=mystor

---
 caps/BasePrincipal.cpp     | 16 ++++++++++++++++
 dom/quota/QuotaManager.cpp | 15 ++++++++-------
 dom/quota/QuotaManager.h   |  2 ++
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp
index 69650396ff7f..f6d58c8268ed 100644
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -19,6 +19,7 @@
 #include "nsServiceManagerUtils.h"
 
 #include "mozilla/dom/CSPDictionariesBinding.h"
+#include "mozilla/dom/quota/QuotaManager.h"
 #include "mozilla/dom/ToJSValue.h"
 #include "mozilla/dom/URLSearchParams.h"
 
@@ -34,6 +35,13 @@ OriginAttributes::CreateSuffix(nsACString& aStr) const
   UniquePtr<URLParams> params(new URLParams());
   nsAutoString value;
 
+  //
+  // Important: While serializing any string-valued attributes, perform a
+  // release-mode assertion to make sure that they don't contain characters that
+  // will break the quota manager when it uses the serialization for file
+  // naming (see addonId below).
+  //
+
   if (mAppId != nsIScriptSecurityManager::NO_APP_ID) {
     value.AppendInt(mAppId);
     params->Set(NS_LITERAL_STRING("appId"), value);
@@ -44,6 +52,7 @@ OriginAttributes::CreateSuffix(nsACString& aStr) const
   }
 
   if (!mAddonId.IsEmpty()) {
+    MOZ_RELEASE_ASSERT(mAddonId.FindCharInSet(dom::quota::QuotaManager::kReplaceChars) == kNotFound);
     params->Set(NS_LITERAL_STRING("addonId"), mAddonId);
   }
 
@@ -60,6 +69,13 @@ OriginAttributes::CreateSuffix(nsACString& aStr) const
     aStr.AppendLiteral("^");
     aStr.Append(NS_ConvertUTF16toUTF8(value));
   }
+
+// In debug builds, check the whole string for illegal characters too (just in case).
+#ifdef DEBUG
+  nsAutoCString str;
+  str.Assign(aStr);
+  MOZ_ASSERT(str.FindCharInSet(dom::quota::QuotaManager::kReplaceChars) == kNotFound);
+#endif
 }
 
 namespace {
diff --git a/dom/quota/QuotaManager.cpp b/dom/quota/QuotaManager.cpp
index 93a516177812..3ecffc3c9809 100644
--- a/dom/quota/QuotaManager.cpp
+++ b/dom/quota/QuotaManager.cpp
@@ -92,6 +92,12 @@ namespace mozilla {
 namespace dom {
 namespace quota {
 
+// We want profiles to be platform-independent so we always need to replace
+// the same characters on every platform. Windows has the most extensive set
+// of illegal characters so we use its FILE_ILLEGAL_CHARACTERS and
+// FILE_PATH_SEPARATOR.
+const char QuotaManager::kReplaceChars[] = CONTROL_CHARACTERS "/:*?\"<>|\\";
+
 namespace {
 
 /*******************************************************************************
@@ -1075,19 +1081,14 @@ public:
 void
 SanitizeOriginString(nsCString& aOrigin)
 {
-  // We want profiles to be platform-independent so we always need to replace
-  // the same characters on every platform. Windows has the most extensive set
-  // of illegal characters so we use its FILE_ILLEGAL_CHARACTERS and
-  // FILE_PATH_SEPARATOR.
-  static const char kReplaceChars[] = CONTROL_CHARACTERS "/:*?\"<>|\\";
 
 #ifdef XP_WIN
-  NS_ASSERTION(!strcmp(kReplaceChars,
+  NS_ASSERTION(!strcmp(QuotaManager::kReplaceChars,
                        FILE_ILLEGAL_CHARACTERS FILE_PATH_SEPARATOR),
                "Illegal file characters have changed!");
 #endif
 
-  aOrigin.ReplaceChar(kReplaceChars, '+');
+  aOrigin.ReplaceChar(QuotaManager::kReplaceChars, '+');
 }
 
 bool
diff --git a/dom/quota/QuotaManager.h b/dom/quota/QuotaManager.h
index 9428afe706e7..2aa5b3279299 100644
--- a/dom/quota/QuotaManager.h
+++ b/dom/quota/QuotaManager.h
@@ -124,6 +124,8 @@ public:
   NS_DECL_NSIQUOTAMANAGER
   NS_DECL_NSIOBSERVER
 
+  static const char kReplaceChars[];
+
   // Returns a non-owning reference.
   static QuotaManager*
   GetOrCreate();