From ab9e2baf22f46695f271b16acddaa57f5a618953 Mon Sep 17 00:00:00 2001 From: Thomas Barber Date: Thu, 13 Jun 2024 08:01:35 +0000 Subject: [PATCH] Foxhound: fixing failing trim tests --- js/src/builtin/String.cpp | 36 ++++++++++++++++-------------- js/src/jit/CodeGenerator.cpp | 12 ++++++++++ js/src/tests/non262/taint/shell.js | 11 +++++---- js/src/tests/non262/taint/trim.js | 20 +++++++++-------- 4 files changed, 49 insertions(+), 30 deletions(-) diff --git a/js/src/builtin/String.cpp b/js/src/builtin/String.cpp index 9bc80caa03774..4ede771a6c0e6 100644 --- 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 index 5ace8e088e0c6..7c3dfb76c462f 100644 --- 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 index 503e7b0c9eac8..d8b19b69436eb 100644 --- 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 index 7f7cf311b4185..32c0c01a599cc 100644 --- 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);