Skip to content

Fixes Array.indexOf/includes for tainted numbers#7

Merged
tmbrbr merged 1 commit into
tmbrbr:primitaint-mergefrom
leeN:primitaint-array-search
Aug 7, 2024
Merged

Fixes Array.indexOf/includes for tainted numbers#7
tmbrbr merged 1 commit into
tmbrbr:primitaint-mergefrom
leeN:primitaint-array-search

Conversation

@leeN

@leeN leeN commented Aug 7, 2024

Copy link
Copy Markdown

Before, Array.indexOf and Array.includes had subtle differences in behavior regarding tainted values or keys when using said functions.

This adds a special case for tainted numbers and unboxes them so that they behave the same. It requires flagging tainted numbers as not bitwise comparable, which might have downstream effects (?).

For a demonstration, see the following code:

let tkey = taint(42);
let key = 42;
function taintedKeyTest() {
	let nums = [1, key, 3];
	let idx_t = nums.indexOf(tkey);
	let idx_ut = nums.indexOf(key);
	let inc_t = nums.includes(tkey);
	let inc_ut = nums.includes(key);
	console.log(`Index of: t[${idx_t}], ut[${idx_ut}]`);
	console.log(`Includes: t[${inc_t}], ut[${inc_ut}]`);
}

function taintedValueTest() {
	let nums = [1, tkey, 3];
	let idx_t = nums.indexOf(tkey);
	let idx_ut = nums.indexOf(key);
	let inc_t = nums.includes(tkey);
	let inc_ut = nums.includes(key);
	console.log(`Index of: t[${idx_t}], ut[${idx_ut}]`);
	console.log(`Includes: t[${inc_t}], ut[${inc_ut}]`);
}
console.log(`Tainted Key:`);
taintedKeyTest();
console.log(`Tainted value:`);
taintedValueTest();

Before, the values did differ for all 4 cases, and now they show the same results.

Before Array.indexOf and Array.includes had subtle differences in
behavior when it came to tainted values or keys when using said
functions.

This adds some special case for tainted numbers and unboxes them, so
that they behave the same. This requires to flag tainted numbers as not
bitwise comparable, which might have downstream effects (?)
@leeN

leeN commented Aug 7, 2024

Copy link
Copy Markdown
Author

This ignores the special cases for nan. I'm not quite sure we can have a tainted nan value and what its semantics shall be. Consequently, I left this out for now.

@tmbrbr tmbrbr merged commit c6599da into tmbrbr:primitaint-merge Aug 7, 2024
@leeN leeN deleted the primitaint-array-search branch August 8, 2024 09:33
tmbrbr pushed a commit that referenced this pull request May 7, 2025
…glandium

We're still constrained by any hard limits applied by the distro, but
empirically this is 1024k on Debian/Ubuntu and 512k on Fedora.  In the
past Fedora's limit was only 4k (see bug 1401776 comment #7); both
that value and the current 512k seem to come from systemd's defaults,
and could be changed via systemd config or be overridden via the file
`/etc/security/limits.conf`.

64k is probably enough given that a lot of fd usage is shared memory,
and we're usually limited to 64k mappings due to the vma limit (`sysctl
vm.max_map_count`).

This patch does not change the limit for macOS, because we mostly use
Mach shared memory which doesn't use file descriptors.

Differential Revision: https://phabricator.services.mozilla.com/D212631
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants