Bug 805121 - Be more careful checking math to avoid incorrect behaviors. r=terrence
authorJeff Walden <jwalden@mit.edu>
Wed, 31 Oct 2012 15:07:59 -0700
changeset 109928 670bd5394fa97ed9190538ce2000eef2a913cae8
parent 109927 2c147a5abc87a24176566efbc34b9181037862b4
child 109929 894976bf2a99d40172d2a04dacf150a384e79041
push id62
push userryanvm@gmail.com
push dateWed, 12 Dec 2012 22:19:24 +0000
reviewersterrence
bugs805121
milestone17.0.1esrpre
Bug 805121 - Be more careful checking math to avoid incorrect behaviors. r=terrence
js/src/jsstr.cpp
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -12,16 +12,17 @@
  * native methods store strings (possibly newborn) converted from their 'this'
  * parameter and arguments on the stack: 'this' conversions at argv[-1], arg
  * conversions at their index (argv[0], argv[1]).  This is a legitimate method
  * of rooting things that might lose their newborn root due to subsequent GC
  * allocations in the same native method.
  */
 
 #include "mozilla/Attributes.h"
+#include "mozilla/CheckedInt.h"
 #include "mozilla/FloatingPoint.h"
 
 #include <stdlib.h>
 #include <string.h>
 #include "jstypes.h"
 #include "jsutil.h"
 #include "jsprf.h"
 #include "jsapi.h"
@@ -57,16 +58,18 @@
 #include "vm/StringObject-inl.h"
 #include "vm/String-inl.h"
 
 using namespace js;
 using namespace js::gc;
 using namespace js::types;
 using namespace js::unicode;
 
+using mozilla::CheckedInt;
+
 static JSLinearString *
 ArgToRootedString(JSContext *cx, CallArgs &args, unsigned argno)
 {
     if (argno >= args.length())
         return cx->runtime->atomState.typeAtoms[JSTYPE_VOID];
 
     Value &arg = args[argno];
     JSString *str = ToString(cx, arg);
@@ -2020,29 +2023,38 @@ FindReplaceLength(JSContext *cx, RegExpS
         rdata.repstr = repstr->ensureLinear(cx);
         if (!rdata.repstr)
             return false;
         *sizep = rdata.repstr->length();
         return true;
     }
 
     JSString *repstr = rdata.repstr;
-    size_t replen = repstr->length();
+    CheckedInt<uint32_t> replen = repstr->length();
     for (const jschar *dp = rdata.dollar, *ep = rdata.dollarEnd; dp;
          dp = js_strchr_limit(dp, '$', ep)) {
         JSSubString sub;
         size_t skip;
         if (InterpretDollar(cx, res, dp, ep, rdata, &sub, &skip)) {
-            replen += sub.length - skip;
+            if (sub.length > skip)
+                replen += sub.length - skip;
+            else
+                replen -= skip - sub.length;
             dp += skip;
         } else {
             dp++;
         }
     }
-    *sizep = replen;
+
+    if (!replen.isValid()) {
+        js_ReportAllocationOverflow(cx);
+        return false;
+    }
+
+    *sizep = replen.value();
     return true;
 }
 
 /*
  * Precondition: |rdata.sb| already has necessary growth space reserved (as
  * derived from FindReplaceLength).
  */
 static void
@@ -2083,18 +2095,24 @@ ReplaceRegExpCallback(JSContext *cx, Reg
     size_t leftoff = rdata.leftIndex;
     size_t leftlen = res->matchStart() - leftoff;
     rdata.leftIndex = res->matchLimit();
 
     size_t replen = 0;  /* silence 'unused' warning */
     if (!FindReplaceLength(cx, res, rdata, &replen))
         return false;
 
-    size_t growth = leftlen + replen;
-    if (!rdata.sb.reserve(rdata.sb.length() + growth))
+    CheckedInt<uint32_t> newlen(rdata.sb.length());
+    newlen += leftlen;
+    newlen += replen;
+    if (!newlen.isValid()) {
+        js_ReportAllocationOverflow(cx);
+        return false;
+    }
+    if (!rdata.sb.reserve(newlen.value()))
         return false;
 
     JSLinearString &str = rdata.str->asLinear();  /* flattened for regexp */
     const jschar *left = str.chars() + leftoff;
 
     rdata.sb.infallibleAppend(left, leftlen); /* skipped-over portion of the search value */
     DoReplace(cx, res, rdata);
     return true;