XS clamps Array.prototype.{findLast,findLastIndex} index values too low

[gibson042]

Environment: XS 13.0.0 32 4

Description
Array.prototype.{findLast,findLastIndex} index clamping does not conform with the ECMAScript specification.

Steps to Reproduce

[
       Array.prototype.findLast.call({ length: 2 ** 53, [2**53 - 2]: "<maximum index>" }, (v, i) => true),
  Array.prototype.findLastIndex.call({ length: 2 ** 53, [2**53 - 2]: "<maximum index>" }, (v, i) => true),
]

test262 pull request: tc39/test262#3775

Actual behavior

[undefined, -1]

Expected behavior

["<maximum index>", 9007199254740990]

Array.prototype.findLast and findLastIndex are both specified to initialize len as LengthOfArrayLike(O), where LengthOfArrayLike uses ToLength to enforce a maximum result of 2⁵³ - 1. Therefore, both methods should be willing to start with an index of up to 2⁵³ - 2 (and find and findIndex should also be willing to iterate up to that point, even though it would take an absurdly long time).

[phoddie]

Thanks for the report. I'm able to reproduce your results. We'll take a look.

[phoddie]

Short answer – There's no immediate intention to address this.

Longer answer - This is a consequence of an implementation limit in XS which uses 32-bit values for indices to minimize RAM use on microcontrollers. A build option could use 64-bit values instead. That would favor conformance over RAM use. Making and verifying those changes is significant work for which a strong motivation doesn't currently exist.

[gibson042]

Hmm, would it be possible to shunt such cases into a slow path? Practical code is exceedingly unlikely to ever hit it and can remain efficient, but you would be able to avoid potentially exploitable spec divergence.

[phoddie]

Hmm, would it be possible to shunt such cases into a slow path?

The existence of such a path is all-but-certain to have performance implications on code that doesn't need it. And it will add complexity to the engine, which works against the goal of having a robust implementation. That slow case would still need to be optional because there's no benefit benefit to embedded uses so there's no benefit in carrying the code. Maybe there's an elegant solution, but it isn't obvious at the moment.

The solution we recommend would have no real cost on 64-bit systems and would likely address issues beyond just this edge case. There would likely be little or no new code to maintain, as it is mostly a change of type. The effort is making those changes, verifying conformance, updating snapshot read/write, checking the changes against various compilers & ports, etc.

But, if you wouldn't mind, please help me understand: what's the "potentially exploitable spec divergence"?

[gibson042]

But, if you wouldn't mind, please help me understand: what's the "potentially exploitable spec divergence"?

Deviation from specified behavior that can allow buggy or malicious code to behave differently across environments (e.g., on a 64-bit developer workstation vs. on a real embedded device), making its detection more difficult and less likely. And willful violations of the spec can in some cases encourage the latter. Proper support of the spec by implementations, even if with degraded performance (but hopefully only significant for edge cases), discourages that and encourages cross-platform code.

[phoddie]

Thank you for the clarification. I understand your point is about the general value of conformance with the standard, not a specific vulnerability caused by this issue.