Skip to content

[VM] Validate toFixed receivers before coercing fractionDigits - #2308

Open
ZainnQureshii wants to merge 1 commit into
meta:static_hfrom
ZainnQureshii:fix/tofixed-receiver
Open

ZainnQureshii wants to merge 1 commit into
meta:static_hfrom
ZainnQureshii:fix/tofixed-receiver

Conversation

@ZainnQureshii

Copy link
Copy Markdown

Summary

Before this change, Number.prototype.toFixed coerces and range-checks fractionDigits before validating its receiver in lib/VM/JSLib/Number.cpp:399. Invalid receivers can therefore run user code and throw Error or RangeError before the required TypeError.

Move the existing receiver check from its old position at lib/VM/JSLib/Number.cpp:417 to :401, before argument coercion at lib/VM/JSLib/Number.cpp:414, matching ECMAScript §21.1.3.3. The regression at test/hermes/number-tofixed-receiver.js:35 changes from receiver 0 Error true RangeError to receiver 0 TypeError false TypeError, with the same result for eight invalid receivers. Primitive and boxed Number controls still print 1.25 1.

This fixes the handwritten implementation only. The generated snapshots API/hermes_sandbox/external/hermes_sandbox_impl_dbg_compiled_5.c:202384 and API/hermes_sandbox/external/hermes_sandbox_impl_opt_compiled_5.c:106173 remain unchanged and retain argument-first validation until regeneration, which API/hermes_sandbox/README.md:5 permits periodically.

Test Plan

ASan Debug build on macOS. Paths below use build for the out-of-tree CMake directory and lit-* for its sibling test output directories.

cmake -S . -B build -G 'Unix Makefiles' \
  -DCMAKE_BUILD_TYPE=Debug -DHERMES_ENABLE_ADDRESS_SANITIZER=ON \
  -DCMAKE_CXX_FLAGS=-O1 -DCMAKE_C_FLAGS=-O1
# exit 0

cmake --build build --target hermes FileCheck shermes \
  shermes_console hermesvm --parallel 8
# exit 0 before the red run and again after restoring the fix

With the C++ fix reverted and the new test kept:

python3 build/bin/hermes-lit -j1 -v \
  --param hermes="$PWD/build/bin/hermes" \
  --param shermes="$PWD/build/bin/shermes" \
  --param FileCheck="$PWD/build/bin/FileCheck" \
  --param asan=ON --param gc=HADES \
  --param test_exec_root="$PWD/lit-red" \
  test/hermes/number-tofixed-receiver.js
# exit 1: 0 passed, 1 failed; receiver 0 Error true RangeError

After restoring the fix and rebuilding:

python3 build/bin/hermes-lit -j1 -v -a \
  --param hermes="$PWD/build/bin/hermes" \
  --param shermes="$PWD/build/bin/shermes" \
  --param FileCheck="$PWD/build/bin/FileCheck" \
  --param asan=ON --param gc=HADES \
  --param test_exec_root="$PWD/lit-green" \
  test/hermes/number-tofixed-receiver.js
# exit 0: 1 passed, 0 failed; source, emitted bytecode and native shermes

python3 build/bin/hermes-lit -j1 -v \
  --param hermes="$PWD/build/bin/hermes" \
  --param shermes="$PWD/build/bin/shermes" \
  --param FileCheck="$PWD/build/bin/FileCheck" \
  --param asan=ON --param gc=HADES \
  --param test_exec_root="$PWD/lit-number" \
  test/hermes/arraySetLength-toNumber.js \
  test/hermes/bigint-tonumber.js \
  test/hermes/flow/builtin-number.js \
  test/hermes/flow/number-literal-index.js \
  test/hermes/intl/number-format-fraction-digits.js \
  test/hermes/intl/number-format-significant-digits.js \
  test/hermes/intl/number-format-unit.js \
  test/hermes/intl/number-format.js \
  test/hermes/number-functions.js \
  test/hermes/regress-intl-number-format-bigint-input.js
# exit 0: 5 passed, 0 failed, 5 unsupported (Intl disabled)

bash utils/format.sh
# exit 0: no changes needed for lib/VM/JSLib/Number.cpp
clang-format --dry-run --Werror lib/VM/JSLib/Number.cpp
# exit 0
git diff --check
# exit 0

The build printed Apple linker warnings about deprecated and ignored -no_pie. utils/tidy.sh (clang-tidy), full check-hermes, release/JIT builds and sandbox execution were not run.

Prepared with AI assistance (Claude Code) and reviewed with GitHub Copilot.

🤖 Generated with Claude Code

Number.prototype.toFixed coerced and range-checked fractionDigits before checking its receiver, so an invalid receiver could run user valueOf code or throw RangeError before the TypeError the spec requires (thisNumberValue is step 1). Move the receiver check first, matching toExponential and toPrecision.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Nov 10, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant