From 13e736d609879cded93f3c6757364787b356b578 Mon Sep 17 00:00:00 2001
From: Henri Sivonen <hsivonen@hsivonen.fi>
Date: Fri, 14 Sep 2018 18:58:15 +0000
Subject: [PATCH] Bug 1490972 - Limit the number of bytes poisoned to avoid
 quadratic behavior. r=froydnj

XPCOM strings mark logically unused parts of nsStringBuffer as uninitialized
in debug builds by writing a marker byte and if memory checking is active,
by telling the memory checking that the range of memory is uninitialized.

This patch limits such marking to up to 16 code units to avoid quadratic
behavior, which is especially bad when there's a large disparity between
length and capacity (after a call to SetCapacity()).

The assumption here is that even a small poisoned memory range is enough
to detect the bugs that the poisoning is intended to detect.

MozReview-Commit-ID: 178rp0ckztj

Differential Revision: https://phabricator.services.mozilla.com/D5838

--HG--
extra : moz-landing-system : lando
---
 xpcom/string/nsTSubstring.cpp | 38 ++++++++++++++++++++++-------------
 xpcom/string/nsTSubstring.h   |  9 ++++++++-
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/xpcom/string/nsTSubstring.cpp b/xpcom/string/nsTSubstring.cpp
index e136024cfbfa..226b9debd7ce 100644
--- a/xpcom/string/nsTSubstring.cpp
+++ b/xpcom/string/nsTSubstring.cpp
@@ -128,14 +128,19 @@ nsTSubstring<T>::StartBulkWriteImpl(size_type aCapacity,
                         this->mData + aOldSuffixStart,
                         aSuffixLength);
       if (aSuffixLength) {
-        char_traits::uninitialize(this->mData + aPrefixToPreserve,
-                                  aNewSuffixStart - aPrefixToPreserve);
-        char_traits::uninitialize(this->mData + aNewSuffixStart + aSuffixLength,
-                                  curCapacity + 1 - aNewSuffixStart -
-                                    aSuffixLength);
+        char_traits::uninitialize(
+          this->mData + aPrefixToPreserve,
+          XPCOM_MIN(size_t(aNewSuffixStart - aPrefixToPreserve),
+                    kNsStringBufferMaxPoison));
+        char_traits::uninitialize(
+          this->mData + aNewSuffixStart + aSuffixLength,
+          XPCOM_MIN(size_t(curCapacity + 1 - aNewSuffixStart - aSuffixLength),
+                    kNsStringBufferMaxPoison));
       } else {
-        char_traits::uninitialize(this->mData + aPrefixToPreserve,
-                                  curCapacity + 1 - aPrefixToPreserve);
+        char_traits::uninitialize(
+          this->mData + aPrefixToPreserve,
+          XPCOM_MIN(size_t(curCapacity + 1 - aPrefixToPreserve),
+                    kNsStringBufferMaxPoison));
       }
       return curCapacity;
     }
@@ -235,14 +240,19 @@ nsTSubstring<T>::StartBulkWriteImpl(size_type aCapacity,
     char_traits::move(
       newData + aNewSuffixStart, oldData + aOldSuffixStart, aSuffixLength);
     if (aSuffixLength) {
-      char_traits::uninitialize(this->mData + aPrefixToPreserve,
-                                aNewSuffixStart - aPrefixToPreserve);
-      char_traits::uninitialize(this->mData + aNewSuffixStart + aSuffixLength,
-                                newCapacity + 1 - aNewSuffixStart -
-                                  aSuffixLength);
+      char_traits::uninitialize(
+        this->mData + aPrefixToPreserve,
+        XPCOM_MIN(size_t(aNewSuffixStart - aPrefixToPreserve),
+                  kNsStringBufferMaxPoison));
+      char_traits::uninitialize(
+        this->mData + aNewSuffixStart + aSuffixLength,
+        XPCOM_MIN(size_t(newCapacity + 1 - aNewSuffixStart - aSuffixLength),
+                  kNsStringBufferMaxPoison));
     } else {
-      char_traits::uninitialize(this->mData + aPrefixToPreserve,
-                                newCapacity + 1 - aPrefixToPreserve);
+      char_traits::uninitialize(
+        this->mData + aPrefixToPreserve,
+        XPCOM_MIN(size_t(newCapacity + 1 - aPrefixToPreserve),
+                  kNsStringBufferMaxPoison));
     }
   } else {
     char_traits::copy(newData, oldData, aPrefixToPreserve);
diff --git a/xpcom/string/nsTSubstring.h b/xpcom/string/nsTSubstring.h
index 96f925dce8a3..80eb674d9648 100644
--- a/xpcom/string/nsTSubstring.h
+++ b/xpcom/string/nsTSubstring.h
@@ -24,6 +24,11 @@
 #error "Using XPCOM strings is limited to code linked into libxul."
 #endif
 
+// The max number of logically uninitialized code units to
+// fill with a marker byte or to mark as unintialized for
+// memory checking. (Limited to avoid quadratic behavior.)
+const size_t kNsStringBufferMaxPoison = 16;
+
 template <typename T> class nsTSubstringSplitter;
 template <typename T> class nsTString;
 template <typename T> class nsTSubstring;
@@ -1393,7 +1398,9 @@ private:
     // counting after the zero terminator the we just wrote above,
     // we end up overwriting the space for terminator not reflected
     // in the capacity number.
-    char_traits::uninitialize(base_string_type::mData + aLength + 1, Capacity() - aLength);
+    char_traits::uninitialize(
+      base_string_type::mData + aLength + 1,
+      XPCOM_MIN(size_t(Capacity() - aLength), kNsStringBufferMaxPoison));
 #endif
   }