镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

buffer: validate end argument of indexOf methods - #66590

Open
gengjiawen wants to merge 1 commit into
nodejs:mainfrom
gengjiawen:buffer-indexof-validate-end
Open

gengjiawen wants to merge 1 commit into
nodejs:mainfrom
gengjiawen:buffer-indexof-validate-end

Conversation

@gengjiawen

Copy link
Copy Markdown
Member

Buffer.prototype.indexOf(), lastIndexOf() and includes() pass any end that is neither a string nor undefined straight to the binding, which CHECKs that it is a number, so buf.indexOf('a', 0, null) aborts the process. Numbers that do not fit in an int64 (Infinity, 1e20) go through an undefined double-to-int64 conversion and the search returns -1 on x64.

This validates end with validateNumber() in bidirectionalIndexOf() and clamps it to [0, buffer.byteLength] before it reaches C++:

  • non-numbers now throw ERR_INVALID_ARG_TYPE instead of aborting
  • Infinity and other values past the end search the whole buffer
  • NaN and -Infinity select an empty range (they already returned -1 before)
  • finite values behave as before, the binding already clamped them to the same range

Fixes: #66589
Refs: #62390

AI disclosure: written with the help of Claude (claude:opus-5.5). I checked the bug against the source and the latest nightly, and reviewed the fix and tests.

Buffer.prototype.indexOf(), lastIndexOf() and includes() passed any
`end` that was neither a string nor undefined straight to the binding,
which CHECKs that it is a number, so values such as `null` or `{}`
aborted the process. Numbers that do not fit in an int64 (`Infinity`,
`1e20`) went through an undefined double-to-int64 conversion and made
the search return -1 on x64.

Throw ERR_INVALID_ARG_TYPE for non-number values and clamp numbers to
[0, buffer.byteLength] before calling into the binding.

Fixes: nodejs#66589
Refs: nodejs#62390
Signed-off-by: Jiawen Geng <technicalcute@gmail.com>
Assisted-by: claude:opus-5.5
@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run. labels Oct 8, 2026
@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.43%. Comparing base (c3189d4) to head (8a5c8ae).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66590      +/-   ##
==========================================
- Coverage   90.45%   90.43%   -0.02%     
==========================================
  Files         791      791              
  Lines      276562   276572      +10     
  Branches    53111    53116       +5     
==========================================
- Hits       250154   250131      -23     
- Misses      16801    16841      +40     
+ Partials     9607     9600       -7     
Files with missing lines Coverage Δ
lib/buffer.js 99.74% <100.00%> (+<0.01%) ⬆️

... and 28 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.

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

Labels

buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buffer#indexOf()/lastIndexOf()/includes() abort the process when end is not a number

2 participants