Merge pull request #280 from leeN/rooting_issues

Added Rooting to avoid GC Hazards

Author: leeN

js/src/builtin/Array.cpp

@@ -1376,7 +1376,8 @@ bool js::array_join(JSContext* cx, unsigned argc, Value* vp) {
   }
 
   // Step 8.
-  JSString* str = sb.finishString();
+  // Foxhound: We have to root the string here, as we introduce the TaintOperationFromContext call, which can trigger the GC.
+  JS::Rooted<JSString*> str(cx, sb.finishString());
   if (!str) {
     return false;
   }

js/src/builtin/JSON.cpp

@@ -2358,14 +2358,16 @@ bool json_stringify(JSContext* cx, unsigned argc, Value* vp) {
   // needs to support returning undefined. So this is a little awkward
   // for the API, because we want to support streaming writers.
   if (!sb.empty()) {
-    JSString* str = sb.finishString();
+    // Foxhound: We have to root the string here, as we introduce the TaintOperationFromContext call, which can trigger the GC.
+    JS::Rooted<JSString*> str(cx, sb.finishString());
     if (!str) {
       return false;
     }
 
     // TaintFox: Add stringify operation to taint flows.
-    str->taint().extend(TaintOperationFromContext(cx, "JSON.stringify", true));
-
+    if(str->isTainted()) {
+      str->taint().extend(TaintOperationFromContext(cx, "JSON.stringify", true));
+    }
     args.rval().setString(str);
   } else {
     args.rval().setUndefined();

js/src/builtin/String.cpp

@@ -526,7 +526,8 @@ static bool str_escape(JSContext* cx, unsigned argc, Value* vp) {
     return true;
   }
 
-  JSString* res = newChars.toString(cx, newLength);
+  // Foxhound: We have to root the string here, as we introduce the TaintOperationFromContext call, which can trigger the GC.
+  JS::Rooted<JSString*> res(cx, newChars.toString(cx, newLength));
   if (!res) {
     return false;
   }
@@ -970,7 +971,7 @@ JSString* js::SubstringKernel(JSContext* cx, HandleString str, int32_t beginInt,
   uint32_t len = lengthInt;
   // TaintFox
   SafeStringTaint newTaint = str->taint().safeSubTaint(begin, begin + len);
-  
+
   /*
    * Optimization for one level deep ropes.
    * This is common for the following pattern:
@@ -1969,7 +1970,8 @@ static bool str_normalize(JSContext* cx, unsigned argc, Value* vp) {
     form = NormalizationForm::NFC;
   } else {
     // Step 4.
-    JSLinearString* formStr = ArgToLinearString(cx, args, 0);
+    // Foxhound: We have to root the string here, as we introduce the TaintOperationFromContext call, which can trigger the GC.
+    JS::Rooted<JSLinearString*> formStr(cx, ArgToLinearString(cx, args, 0));
     if (!formStr) {
       return false;
     }
@@ -3311,8 +3313,8 @@ static JSLinearString* TrimString(JSContext* cx, JSString* str, bool trimStart,
     TrimString(linear->twoByteChars(nogc), trimStart, trimEnd, length, &begin,
                &end);
   }
-
-  JSLinearString* result = NewDependentString(cx, linear, begin, end - begin);
+  // Foxhound: We have to root the string here, as we introduce the TaintOperationFromContext call, which can trigger the GC.
+  JS::Rooted<JSLinearString*> result(cx, NewDependentString(cx, linear, begin, end - begin));
 
   // TaintFox: Add trim operation to current taint flow.
   // the acutal trimming of taint ranges has been done in
@@ -4292,8 +4294,8 @@ static ArrayObject* CharSplitHelper(JSContext* cx, Handle<JSLinearString*> str,
   splits->ensureDenseInitializedLength(0, resultlen);
 
   for (size_t i = 0; i < resultlen; ++i) {
-    // TaintFox: code modified to avoid atoms.
-    JSString* sub = NewDependentString(cx, str, i, 1);
+    // TaintFox: code modified to avoid atoms, and added rooting because TaintLocationFromContext can trigger a GC.
+    JS::Rooted<JSString*> sub(cx, NewDependentString(cx, str, i, 1));
     // was:
     // JSString* sub = staticStrings.getUnitStringForElement(cx, str, i);
     if (!sub) {

js/src/jit/VMFunctions.cpp

@@ -1356,8 +1356,9 @@ JSString* StringReplace(JSContext* cx, HandleString string,
   MOZ_ASSERT(string);
   MOZ_ASSERT(pattern);
   MOZ_ASSERT(repl);
-  // Foxhound: this will propagate the taint but not add the operation
-  JSString* str = str_replace_string_raw(cx, string, pattern, repl);
+  // Foxhound: this will propagate the taint but not add the operation.
+  // Foxhound: We have to root the string here, as we introduce the TaintOperationFromContext call, which can trigger the GC.
+  Rooted<JSString*> str(cx, str_replace_string_raw(cx, string, pattern, repl));
   if (str && str->taint().hasTaint()) {
     str->taint().extend(TaintOperationFromContext(cx, "replace", true, pattern, repl));
   }

js/src/jsapi.cpp

@@ -5014,7 +5014,7 @@ JS_ReportTaintSink(JSContext* cx, JS::HandleString str, const char* sink, JS::Ha
   // slot of the current global object.
   RootedFunction report(cx);
 
-  JSObject* global = cx->global();
+  JS::Rooted<JSObject*> global(cx, cx->global());
 
   RootedValue slot(cx, JS::GetReservedSlot(global, TAINT_REPORT_FUNCTION_SLOT));
   if (slot.isUndefined()) {

js/src/jstaint.cpp

@@ -417,7 +417,7 @@ void JS::WriteTaintToFile(JSContext* cx, JSString* str, HandleValue location) {
 
   char suffix_path[2048] = {0};
   SprintfLiteral(suffix_path, "%s.%d.%u.json", filename, getpid(), counter++);
-  
+
   Fprinter output;
   if (!output.init(suffix_path)) {
     SEprinter p;
@@ -504,7 +504,7 @@ void JS::PrintJsonTaint(JSContext* cx, JSString* str, HandleValue location, js::
 
   // Dump additional information from the taintreport
   if (location.isObject()) {
-    JSObject* obj = ToObject(cx, location);
+    JS::Rooted<JSObject*> obj(cx, ToObject(cx, location));
     PrintJsonObject(cx, obj, json);
   }