Skip to content

Commit

Permalink
Foxhound: unbox tainted numbers used as map keys
Browse files Browse the repository at this point in the history
  • Loading branch information
tmbrbr committed Jul 30, 2024
1 parent 301b663 commit af544cf
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 1 deletion.
11 changes: 11 additions & 0 deletions js/src/builtin/MapObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,17 @@ bool HashableValue::setValue(JSContext* cx, HandleValue v) {
value = v;
}

// Additional check to unbox if we have a tainted Number
// Required to fix breakage with playwright integration
// But only if tainted as default JavaScript behaviour is that
// primitive and NumberObject keys are distinct.
if (value.isObject() && value.toObject().is<NumberObject>()) {
NumberObject& number = value.toObject().as<NumberObject>();
if (number.taint()) {
value = NumberValue(number.unbox());
}
}

MOZ_ASSERT(value.isUndefined() || value.isNull() || value.isBoolean() ||
value.isNumber() || value.isString() || value.isSymbol() ||
value.isObject() || value.isBigInt() ||
Expand Down
15 changes: 14 additions & 1 deletion js/src/tests/non262/taint/maps.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
function mapNumberObjectKeyTest() {
let map = new Map();
let tainted = new Number(42);
let untainted = 42;
let value = "foo";
map.set(untainted, value);
let ret_val_untainted = map.get(untainted)
let ret_val_tainted = map.get(tainted)
// Rather unintuitively, Number object and primitive keys are treated differently in JavaScript
assertNotEq(ret_val_tainted, ret_val_untainted);
}

function mapTaintedKeyTest() {
let map = new Map();
let tainted = taint(42);
Expand All @@ -20,7 +32,8 @@ function mapUntaintedKeyTest() {
assertEq(ret_val_tainted, ret_val_untainted);
}

runTaintTest(mapTaintTest);
runTaintTest(mapNumberObjectKeyTest);
runTaintTest(mapTaintedKeyTest);
runTaintTest(mapUntaintedKeyTest);


Expand Down
11 changes: 11 additions & 0 deletions js/src/tests/shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,17 @@
global.assertEq = assertEq;
}

var assertNotEq = global.assertNotEq;
if (typeof assertNotEq !== "function") {
assertNotEq = function assertNotEq(actual, expected, message) {
if (SameValue(actual, expected)) {
throw new TypeError(`Assertion failed: got "${actual}", expected "${expected}"` +
(message ? ": " + message : ""));
}
};
global.assertNotEq = assertNotEq;
}

function assertEqArray(actual, expected) {
var len = actual.length;
assertEq(len, expected.length, "mismatching array lengths");
Expand Down

0 comments on commit af544cf

Please sign in to comment.