From 927c25ab9b1c353c07d5a1b4e56d8f74fff62949 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 12 Apr 2026 22:16:34 +0200 Subject: [PATCH] buffer: fix end parameter bugs in indexOf/lastIndexOf - Fix FastIndexOfNumber parameter order mismatch (end_i64 and is_forward were swapped vs the JS call site and slow path) - Clamp negative end values to 0 to prevent size_t overflow in IndexOfString, IndexOfBuffer, and IndexOfNumberImpl - Clamp empty needle result to search_end Signed-off-by: Robert Nagy Co-Authored-By: Claude Opus 4.6 (1M context) --- src/node_buffer.cc | 28 +++++++++++++--------- test/parallel/test-buffer-indexof.js | 35 ++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 4d007076602793..b02c643cca1465 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -983,8 +983,8 @@ void IndexOfString(const FunctionCallbackInfo& args) { if (!StringBytes::Size(isolate, needle, enc).To(&needle_length)) return; // search_end is the exclusive upper bound of the search range. - size_t search_end = static_cast( - std::min(end_i64, static_cast(haystack_length))); + size_t search_end = static_cast(std::min( + std::max(end_i64, int64_t{0}), static_cast(haystack_length))); if (enc == UCS2) search_end &= ~static_cast(1); int64_t opt_offset = IndexOfOffset(haystack_length, @@ -993,8 +993,11 @@ void IndexOfString(const FunctionCallbackInfo& args) { is_forward); if (needle_length == 0) { - // Match String#indexOf() and String#lastIndexOf() behavior. - args.GetReturnValue().Set(static_cast(opt_offset)); + // Match String#indexOf() and String#lastIndexOf() behavior, + // but clamp to search_end. + int64_t clamped = + std::min(opt_offset, static_cast(search_end)); + args.GetReturnValue().Set(static_cast(clamped)); return; } @@ -1108,8 +1111,8 @@ void IndexOfBuffer(const FunctionCallbackInfo& args) { const size_t needle_length = needle_contents.length(); // search_end is the exclusive upper bound of the search range. - size_t search_end = static_cast( - std::min(end_i64, static_cast(haystack_length))); + size_t search_end = static_cast(std::min( + std::max(end_i64, int64_t{0}), static_cast(haystack_length))); if (enc == UCS2) search_end &= ~static_cast(1); int64_t opt_offset = IndexOfOffset(haystack_length, @@ -1118,8 +1121,11 @@ void IndexOfBuffer(const FunctionCallbackInfo& args) { is_forward); if (needle_length == 0) { - // Match String#indexOf() and String#lastIndexOf() behavior. - args.GetReturnValue().Set(static_cast(opt_offset)); + // Match String#indexOf() and String#lastIndexOf() behavior, + // but clamp to search_end. + int64_t clamped = + std::min(opt_offset, static_cast(search_end)); + args.GetReturnValue().Set(static_cast(clamped)); return; } @@ -1184,8 +1190,8 @@ int32_t IndexOfNumberImpl(Local buffer_obj, } size_t offset = static_cast(opt_offset); // search_end is the exclusive upper bound of the search range. - size_t search_end = static_cast( - std::min(end_i64, static_cast(buffer_length))); + size_t search_end = static_cast(std::min( + std::max(end_i64, int64_t{0}), static_cast(buffer_length))); const void* ptr; if (is_forward) { @@ -1222,8 +1228,8 @@ int32_t FastIndexOfNumber(Local, Local buffer_obj, uint32_t needle, int64_t offset_i64, - int64_t end_i64, bool is_forward, + int64_t end_i64, // NOLINTNEXTLINE(runtime/references) FastApiCallbackOptions& options) { HandleScope scope(options.isolate); diff --git a/test/parallel/test-buffer-indexof.js b/test/parallel/test-buffer-indexof.js index 21fcd78477ab06..5cefedc21330ac 100644 --- a/test/parallel/test-buffer-indexof.js +++ b/test/parallel/test-buffer-indexof.js @@ -691,3 +691,38 @@ assert.strictEqual(reallyLong.lastIndexOf(pattern), 0); assert.strictEqual(buf.includes('c'), true); } + +{ + const buf = Buffer.from('abcabc'); + + // negative end should be treated as 0 (no match possible). + assert.strictEqual(buf.indexOf('a', 0, -1), -1); + assert.strictEqual(buf.indexOf('a', 0, -100), -1); + assert.strictEqual(buf.indexOf(0x61, 0, -1), -1); + assert.strictEqual(buf.lastIndexOf('a', 5, -1), -1); + assert.strictEqual(buf.lastIndexOf(0x61, 5, -1), -1); + assert.strictEqual(buf.includes('a', 0, -1), false); + assert.strictEqual(buf.indexOf(Buffer.from('a'), 0, -1), -1); + assert.strictEqual(buf.lastIndexOf(Buffer.from('a'), 5, -1), -1); + + // end = 0 means empty search range. + assert.strictEqual(buf.indexOf('a', 0, 0), -1); + assert.strictEqual(buf.indexOf(0x61, 0, 0), -1); + assert.strictEqual(buf.lastIndexOf('a', 5, 0), -1); + assert.strictEqual(buf.lastIndexOf(0x61, 5, 0), -1); + + // end greater than buffer length should be clamped. + assert.strictEqual(buf.indexOf('c', 0, 100), 2); + assert.strictEqual(buf.indexOf(0x63, 0, 100), 2); + assert.strictEqual(buf.lastIndexOf('c', 5, 100), 5); + assert.strictEqual(buf.lastIndexOf(0x63, 5, 100), 5); + assert.strictEqual(buf.indexOf(Buffer.from('c'), 0, 100), 2); + + // empty needle with end parameter should clamp to search_end. + assert.strictEqual(buf.indexOf('', 0, 3), 0); + assert.strictEqual(buf.indexOf('', 5, 3), 3); + assert.strictEqual(buf.indexOf(Buffer.from(''), 5, 3), 3); + assert.strictEqual(buf.indexOf('', 0, 0), 0); + assert.strictEqual(buf.lastIndexOf('', 5, 3), 3); + assert.strictEqual(buf.lastIndexOf(Buffer.from(''), 5, 3), 3); +}