Skip to content

ffi: validate fast integer argument ranges#64614

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-v8-32-bit-int
Open

ffi: validate fast integer argument ranges#64614
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-v8-32-bit-int

Conversation

@trivikr

@trivikr trivikr commented Jul 20, 2026

Copy link
Copy Markdown
Member

Fixes: #64613

Fast FFI calls bypassed the integer range validation performed by the generic FFI path.
Out-of-range i8, u8, i16, and u16 arguments were silently truncated, while out-of-range 64-bit BigInts were wrapped.

This change validates affected integer arguments in the existing JavaScript fast-call wrapper before entering the native trampoline. Invalid values now throw ERR_INVALID_ARG_VALUE, matching the generic path, while valid calls continue to use the Fast API.

The fix also covers bool, char, and unsigned 64-bit arguments, which use the same conversions.


Assisted-by: openai:gpt-5.6-sol

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run. labels Jul 20, 2026
Validate narrow integer and 64-bit BigInt arguments before entering
the Fast API trampoline. This prevents out-of-range values from being
silently truncated or wrapped and matches the generic FFI path.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.6-sol
@trivikr
trivikr force-pushed the ffi-v8-32-bit-int branch from 91a79a2 to 40d2aa3 Compare July 20, 2026 06:51
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.32394% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.13%. Comparing base (00917ba) to head (40d2aa3).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/ffi/fast-api.js 89.65% 5 Missing and 1 partial ⚠️
src/ffi/fast.cc 66.66% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64614      +/-   ##
==========================================
- Coverage   90.14%   90.13%   -0.01%     
==========================================
  Files         741      741              
  Lines      242076   242134      +58     
  Branches    45558    45583      +25     
==========================================
+ Hits       218216   218258      +42     
+ Misses      15385    15379       -6     
- Partials     8475     8497      +22     
Files with missing lines Coverage Δ
src/ffi/fast.h 100.00% <ø> (ø)
src/node_ffi.cc 71.09% <100.00%> (+0.03%) ⬆️
src/ffi/fast.cc 66.27% <66.66%> (+0.02%) ⬆️
lib/internal/ffi/fast-api.js 87.34% <89.65%> (+13.07%) ⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 20, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 20, 2026
@nodejs-github-bot

This comment was marked as outdated.

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 20, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: Fast API calls silently truncate out-of-range integer arguments

4 participants