Foxhound: fixing failing trim tests

SAP · Jun 13, 2024 · ab9e2ba · ab9e2ba
1 parent a24f1f3
commit ab9e2ba
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 30 deletions.
diff --git a/js/src/builtin/String.cpp b/js/src/builtin/String.cpp
@@ -3192,7 +3192,25 @@ static JSLinearString* TrimString(JSContext* cx, JSString* str, bool trimStart,
                &end);
   }
 
-  return NewDependentString(cx, linear, begin, end - begin);
+  JSLinearString* result = NewDependentString(cx, linear, begin, end - begin);
+
+  // TaintFox: Add trim operation to current taint flow.
+  // the acutal trimming of taint ranges has been done in
+  // NewDependentString (StringType-inl.h, JSDependentString::init)
+  if (result && result->isTainted()) {
+    AutoCheckCannotGC nogc;
+    if (trimStart && trimEnd) {
+      result->taint().extend(TaintOperationFromContext(cx, "trim", true));
+    } else if (trimStart) {
+      result->taint().extend(TaintOperationFromContext(cx, "trimStart", true));
+    } else if (trimEnd) {
+      result->taint().extend(TaintOperationFromContext(cx, "trimEnd", true));
+    } else {
+      result->taint().extend(TaintOperationFromContext(cx, "trim", true));
+    }
+  }
+
+  return result;
 }
 
 JSString* js::StringTrim(JSContext* cx, HandleString string) {
@@ -3219,22 +3237,6 @@ static bool TrimString(JSContext* cx, const CallArgs& args, const char* funName,
     return false;
   }
 
-  // TaintFox: Add trim operation to current taint flow.
-  // the acutal trimming of taint ranges has been done in
-  // NewDependentString (StringType-inl.h, JSDependentString::init)
-  if (result->isTainted()) {
-    AutoCheckCannotGC nogc;
-    if (trimStart && trimEnd) {
-      result->taint().extend(TaintOperationFromContext(cx, "trim", true));
-    } else if (trimStart) {
-      result->taint().extend(TaintOperationFromContext(cx, "trimLeft", true));
-    } else if (trimEnd) {
-      result->taint().extend(TaintOperationFromContext(cx, "trimRight", true));
-    } else {
-      result->taint().extend(TaintOperationFromContext(cx, "trim", true));
-    }
-  }
-
   args.rval().setString(result);
   return true;
 }

diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
@@ -13056,6 +13056,12 @@ void CodeGenerator::visitStringTrimStartIndex(LStringTrimStartIndex* lir) {
   Register string = ToRegister(lir->string());
   Register output = ToRegister(lir->output());
 
+  // TaintFox: if we detect a tainted string argument we bail out to the interpreter.
+  bailoutCmpPtr(Assembler::NotEqual,
+                Address(string, JSString::offsetOfTaint()),
+                ImmPtr(nullptr),
+                lir->snapshot());
+
   auto volatileRegs = liveVolatileRegs(lir);
   volatileRegs.takeUnchecked(output);
 
@@ -13075,6 +13081,12 @@ void CodeGenerator::visitStringTrimEndIndex(LStringTrimEndIndex* lir) {
   Register start = ToRegister(lir->start());
   Register output = ToRegister(lir->output());
 
+  // TaintFox: if we detect a tainted string argument we bail out to the interpreter.
+  bailoutCmpPtr(Assembler::NotEqual,
+                Address(string, JSString::offsetOfTaint()),
+                ImmPtr(nullptr),
+                lir->snapshot());
+
   auto volatileRegs = liveVolatileRegs(lir);
   volatileRegs.takeUnchecked(output);
 

diff --git a/js/src/tests/non262/taint/shell.js b/js/src/tests/non262/taint/shell.js
@@ -222,22 +222,25 @@ if (typeof assertNotHasTaintOperation === 'undefined') {
 
 if (typeof assertLastTaintOperationEquals === 'undefined') {
     var assertLastTaintOperationEquals = function(str, opName) {
+        var lastOp = "Unknown";
         for (var i = 0; i < str.taint.length; i++) {
             var range = str.taint[i];
 
             // Quirk: ignore "function call arguments" nodes for now...
             var index = 0;
             while (index < range.flow.length &&
-		   range.flow[index].operation == "function" &&
-		   range.flow[index].arguments[0].startsWith("assert"))
+		        range.flow[index].operation == "function" &&
+		        range.flow[index].arguments[0].startsWith("assert")) {
                 index++;
+            }
 
             var node = range.flow[index];
             if (node.operation === opName) {
                 return true;
             }
+            lastOp = node.operation;
         }
-        throw Error("String '" + str + "' does not contain \"" + opName + "\" as last taint operation. Taint: " + JSON.stringify(str.taint));
+        throw Error("String '" + str + "' does not contain \"" + opName + "\" as last taint operation (\"" + lastOp + "\"). Taint: " + JSON.stringify(str.taint));
     }
 }
 
@@ -247,7 +250,7 @@ if (typeof runTaintTest === 'undefined') {
         // Separate function so it's visible in the backtrace
         var runJITTest = function(doTest) {
             // Force JIT compilation
-            for (var i = 0; i < 100; i++) {
+            for (var i = 0; i < 1000; i++) {
                 //console.log(i);
                 doTest();
             }

diff --git a/js/src/tests/non262/taint/trim.js b/js/src/tests/non262/taint/trim.js
@@ -7,13 +7,13 @@ function trimTaintTest() {
     assertEqualTaint(trimMe.trim(), str);
     assertNotHasTaintOperation(trimMe, 'trim');
 
-    assertLastTaintOperationEquals(trimMe.trimLeft(), 'trimLeft');
+    assertLastTaintOperationEquals(trimMe.trimStart(), 'trimStart');
     assertEqualTaint(trimMe.trimLeft(), str+rpad);
-    assertNotHasTaintOperation(trimMe, 'trimLeft');
+    assertNotHasTaintOperation(trimMe, 'trimStart');
 
-    assertLastTaintOperationEquals(trimMe.trimRight(), 'trimRight');
+    assertLastTaintOperationEquals(trimMe.trimEnd(), 'trimEnd');
     assertEqualTaint(trimMe.trimRight(), lpad+str);
-    assertNotHasTaintOperation(trimMe, 'trimRight');
+    assertNotHasTaintOperation(trimMe, 'trimEnd');
 }
 
 function trimLeftTaintTest() {
@@ -22,9 +22,10 @@ function trimLeftTaintTest() {
   var rpad = "  " + taint("           ") + " ";
   var trimMe = lpad + str + rpad;
 
-  assertLastTaintOperationEquals(trimMe.trimLeft(), 'trimLeft');
+  // trimLeft is now deprecated and just redirected to trimStart
+  assertLastTaintOperationEquals(trimMe.trimLeft(), 'trimStart');
   assertEqualTaint(trimMe.trimLeft(), str+rpad);
-  assertNotHasTaintOperation(trimMe, 'trimLeft');
+  assertNotHasTaintOperation(trimMe, 'trimStart');
 }
 
 function trimRightTaintTest() {
@@ -33,9 +34,10 @@ function trimRightTaintTest() {
   var rpad = "  " + taint("           ") + " ";
   var trimMe = lpad + str + rpad;
 
-  assertLastTaintOperationEquals(trimMe.trimRight(), 'trimRight');
+  // trimRight is now deprecated and just redirected to trimEnd
+  assertLastTaintOperationEquals(trimMe.trimRight(), 'trimEnd');
   assertEqualTaint(trimMe.trimRight(), lpad+str);
-  assertNotHasTaintOperation(trimMe, 'trimRight');
+  assertNotHasTaintOperation(trimMe, 'trimEnd');
 }
 
 function trimStartTaintTest() {
@@ -60,7 +62,7 @@ function trimEndTaintTest() {
   assertNotHasTaintOperation(trimMe, 'trimEnd');
 }
 
-//runTaintTest(trimTaintTest);
+runTaintTest(trimTaintTest);
 runTaintTest(trimLeftTaintTest);
 runTaintTest(trimRightTaintTest);
 runTaintTest(trimStartTaintTest);