From 1a38ec47107f07f0f5c9c6f0b208a1bcd300c764 Mon Sep 17 00:00:00 2001
From: Henri Sivonen <hsivonen@hsivonen.fi>
Date: Fri, 7 Sep 2018 05:47:57 +0000
Subject: [PATCH] Bug 1486711 - Fill logically uninitialized parts of an XPCOM
 string's buffer with a marker byte in debug builds. r=froydnj

MozReview-Commit-ID: IwLikJpacAW

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

--HG--
extra : moz-landing-system : lando
---
 servo/support/gecko/nsstring/src/lib.rs | 14 +++++++++++
 xpcom/string/nsCharTraits.h             | 17 +++++++++++++
 xpcom/string/nsTSubstring.cpp           | 32 +++++++++++++++++++++++++
 3 files changed, 63 insertions(+)

diff --git a/servo/support/gecko/nsstring/src/lib.rs b/servo/support/gecko/nsstring/src/lib.rs
index baa44030118d..7482c088bcd6 100644
--- a/servo/support/gecko/nsstring/src/lib.rs
+++ b/servo/support/gecko/nsstring/src/lib.rs
@@ -452,6 +452,20 @@ macro_rules! define_string_types {
                     let mut this = self.string.as_repr();
                     this.as_mut().length = length as u32;
                     *(this.as_mut().data.as_ptr().offset(length as isize)) = 0;
+                    if cfg!(debug_assertions) {
+                        // Overwrite the unused part in debug builds. Note
+                        // that capacity doesn't include space for the zero
+                        // terminator, so starting after the zero-terminator
+                        // we wrote ends up overwriting the terminator space
+                        // not reflected in the capacity number.
+                        // write_bytes() takes care of multiplying the length
+                        // by the size of T.
+                        ptr::write_bytes(this.as_mut().data.as_ptr().offset((length + 1) as isize),
+                                         0xE4u8,
+                                         self.capacity - length);
+                    }
+                    // We don't have a Rust interface for mozilla/MemoryChecking.h,
+                    // so let's just not communicate with MSan/Valgrind here.
                 }
                 mem::forget(self); // Don't run the failure path in drop()
                 BulkWriteOk{}
diff --git a/xpcom/string/nsCharTraits.h b/xpcom/string/nsCharTraits.h
index 545d5a4d778c..d8f72c8500dc 100644
--- a/xpcom/string/nsCharTraits.h
+++ b/xpcom/string/nsCharTraits.h
@@ -9,6 +9,7 @@
 
 #include <ctype.h> // for |EOF|, |WEOF|
 #include <string.h> // for |memcpy|, et al
+#include "mozilla/MemoryChecking.h"
 
 // This file may be used (through nsUTF8Utils.h) from non-XPCOM code, in
 // particular the standalone software updater. In that case stub out
@@ -148,6 +149,14 @@ struct nsCharTraits<char16_t>
                                           aN * sizeof(char_type)));
   }
 
+  static void uninitialize(char_type* aStr, size_t aN)
+  {
+#ifdef DEBUG
+    memset(aStr, 0xE4, aN * sizeof(char_type));
+#endif
+    MOZ_MAKE_MEM_UNDEFINED(aStr, aN * sizeof(char_type));
+  }
+
   static char_type*
   copyASCII(char_type* aStr1, const char* aStr2, size_t aN)
   {
@@ -356,6 +365,14 @@ struct nsCharTraits<char>
                                           aN * sizeof(char_type)));
   }
 
+  static void uninitialize(char_type* aStr, size_t aN)
+  {
+#ifdef DEBUG
+    memset(aStr, 0xE4, aN * sizeof(char_type));
+#endif
+    MOZ_MAKE_MEM_UNDEFINED(aStr, aN * sizeof(char_type));
+  }
+
   static char_type*
   copyASCII(char_type* aStr1, const char* aStr2, size_t aN)
   {
diff --git a/xpcom/string/nsTSubstring.cpp b/xpcom/string/nsTSubstring.cpp
index cf98aed96e50..30f22a5aa888 100644
--- a/xpcom/string/nsTSubstring.cpp
+++ b/xpcom/string/nsTSubstring.cpp
@@ -127,6 +127,16 @@ nsTSubstring<T>::StartBulkWriteImpl(size_type aCapacity,
       char_traits::move(this->mData + aNewSuffixStart,
                         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);
+      } else {
+        char_traits::uninitialize(this->mData + aPrefixToPreserve,
+                                  curCapacity + 1 - aPrefixToPreserve);
+      }
       return curCapacity;
     }
   }
@@ -224,6 +234,16 @@ nsTSubstring<T>::StartBulkWriteImpl(size_type aCapacity,
   if (oldData == newData) {
     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);
+    } else {
+      char_traits::uninitialize(this->mData + aPrefixToPreserve,
+                                newCapacity + 1 - aPrefixToPreserve);
+    }
   } else {
     char_traits::copy(newData, oldData, aPrefixToPreserve);
     char_traits::copy(
@@ -242,6 +262,18 @@ nsTSubstring<T>::FinishBulkWriteImpl(size_type aLength)
   if (aLength) {
     this->mData[aLength] = char_type(0);
     this->mLength = aLength;
+#ifdef DEBUG
+    // ifdefed in order to avoid the call to Capacity() in non-debug
+    // builds.
+    //
+    // Our string is mutable, so Capacity() doesn't return zero.
+    // Capacity() doesn't include the space for the zero terminator,
+    // but we want to unitialize that slot, too. Since we start
+    // 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(this->mData + aLength + 1, Capacity() - aLength);
+#endif
   } else {
     ::ReleaseData(this->mData, this->mDataFlags);
     SetToEmptyBuffer();