Bug 1509542 part 1 - Fix Escape to not rely on result.length <= JSString::MAX_LENGTH. r=jwalden
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 26 Nov 2018 23:23:41 +0000
changeset 507454 e6867843444fdb9f9c0957d0548ff2d4dfe051f2
parent 507453 fbe3f2183f32134a867db07f07352fb33428ea3c
child 507455 b3f239eccb59e2fe9632ee1106dfd78a176200c1
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1509542
milestone65.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 1509542 part 1 - Fix Escape to not rely on result.length <= JSString::MAX_LENGTH. r=jwalden Differential Revision: https://phabricator.services.mozilla.com/D12877
js/src/builtin/String.cpp
--- a/js/src/builtin/String.cpp
+++ b/js/src/builtin/String.cpp
@@ -131,25 +131,32 @@ Escape(JSContext* cx, const CharT* chars
     /* Take a first pass and see how big the result string will need to be. */
     uint32_t newLength = length;
     for (size_t i = 0; i < length; i++) {
         char16_t ch = chars[i];
         if (ch < 128 && shouldPassThrough[ch]) {
             continue;
         }
 
+        /*
+         * newlength is incremented below by at most 5 and at this point it must
+         * be a valid string length, so this should never overflow uint32_t.
+         */
+        static_assert(JSString::MAX_LENGTH < UINT32_MAX - 5,
+                      "Adding 5 to valid string length should not overflow");
+
+        MOZ_ASSERT(newLength <= JSString::MAX_LENGTH);
+
         /* The character will be encoded as %XX or %uXXXX. */
         newLength += (ch < 256) ? 2 : 5;
 
-        /*
-         * newlength is incremented by at most 5 on each iteration, so worst
-         * case newlength == length * 6. This can't overflow.
-         */
-        static_assert(JSString::MAX_LENGTH < UINT32_MAX / 6,
-                      "newlength must not overflow");
+        if (MOZ_UNLIKELY(newLength > JSString::MAX_LENGTH)) {
+            ReportAllocationOverflow(cx);
+            return false;
+        }
     }
 
     if (newLength == length) {
         *newLengthOut = newLength;
         return true;
     }
 
     if (!newChars.maybeAlloc(cx, newLength)) {