Skip to content

buffer: prevent assertion crash in indexOf when encoding is non-numeric - #67050

Open
nikolas-sapa wants to merge 1 commit into
nodejs:mainfrom
nikolas-sapa:fix/buffer-indexof-crash
Open

nikolas-sapa wants to merge 1 commit into
nodejs:mainfrom
nikolas-sapa:fix/buffer-indexof-crash

Conversation

@nikolas-sapa

Copy link
Copy Markdown

When calling buf.indexOf(val, offset, encoding) (or lastIndexOf/includes) with a non-string and non-numeric value for encoding (such as null or {}), the end argument is not normalized to this.length because the current check only handles typeof end === 'string' or end === undefined:

Buffer.prototype.indexOf = function indexOf(val, offset, end, encoding) {
  if (typeof end === 'string') {
    encoding = end;
    end = this.length;
  } else if (end === undefined) {
    end = this.length;
  }
  return bidirectionalIndexOf(this, val, offset, end, encoding, 0);
};

When encoding is passed as null in a 3-argument call (e.g. buf.indexOf(1, 0, null) or buf.indexOf(Buffer.from('a'), 0, null)), end is passed as null to the C++ bindings. This triggers fatal CHECK assertion failures (CHECK(args[end_idx]->IsNumber()) in src/node_buffer.cc), aborting the process with SIGABRT (exit code 134):

# void node::Buffer::(anonymous namespace)::IndexOfString(const FunctionCallbackInfo<Value> &) at ../src/node_buffer.cc:1036
# Assertion failed: args[5]->IsNumber()

Changes

  1. In Buffer.prototype.indexOf, lastIndexOf, and includes, normalize end to this.length when encoding === undefined && typeof end !== 'number'.
  2. In bidirectionalIndexOf, defensively normalize non-numeric end to a number, defaulting NaN to buffer.length.
  3. Added parallel regression tests in test/parallel/test-buffer-indexof-null-encoding.js covering numeric, buffer, and string targets with null, {}, and false encodings across indexOf, lastIndexOf, and includes.

@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 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

Caution

AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context.

When buf.indexOf(val, offset, encoding) or lastIndexOf/includes was
called with a non-string/non-numeric value (such as null or an object)
for encoding, end was left as that value instead of defaulting to
buffer length. This caused C++ CHECK assertions
(args[end_idx]->IsNumber()) to fail and crash the process with SIGABRT.

Normalize non-numeric 3rd arguments to buffer length and ensure end is
coerced to a number in bidirectionalIndexOf.

Signed-off-by: Nik <84yk8btb9f@privaterelay.appleid.com>
@nikolas-sapa
nikolas-sapa force-pushed the fix/buffer-indexof-crash branch from fd4dc5c to 6ff7f25 Compare October 27, 2026 08:50
@nikolas-sapa

Copy link
Copy Markdown
Author

Could a collaborator trigger CI on this when convenient? Small fix with a regression test included.

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

Labels

agentscan:automated-account 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.

2 participants