Skip to content

Commit 4220cb9

Browse files
committed
buffer: fix lastIndexOf crash for overlong needle
Return -1 in `Buffer.lastIndexOf` if the needle is longer than the haystack. The previous check only tested the corresponding condition for forward searches. This applies only to Node.js v6, as `lastIndexOf` was added in it. Fixes: nodejs#6510
1 parent 8ec2062 commit 4220cb9

File tree

2 files changed

+27
-2
lines changed

2 files changed

+27
-2
lines changed

src/node_buffer.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,7 +1011,8 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
10111011
}
10121012
size_t offset = static_cast<size_t>(opt_offset);
10131013
CHECK_LT(offset, haystack_length);
1014-
if (is_forward && needle_length + offset > haystack_length) {
1014+
if ((is_forward && needle_length + offset > haystack_length) ||
1015+
needle_length > haystack_length) {
10151016
return args.GetReturnValue().Set(-1);
10161017
}
10171018

@@ -1113,7 +1114,8 @@ void IndexOfBuffer(const FunctionCallbackInfo<Value>& args) {
11131114
}
11141115
size_t offset = static_cast<size_t>(opt_offset);
11151116
CHECK_LT(offset, haystack_length);
1116-
if (is_forward && needle_length + offset > haystack_length) {
1117+
if ((is_forward && needle_length + offset > haystack_length) ||
1118+
needle_length > haystack_length) {
11171119
return args.GetReturnValue().Set(-1);
11181120
}
11191121

test/parallel/test-buffer-indexof.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,12 +354,35 @@ assert.equal(b.lastIndexOf('b', {}), 1);
354354
assert.equal(b.lastIndexOf('b', []), -1);
355355
assert.equal(b.lastIndexOf('b', [2]), 1);
356356

357+
// Test needles longer than the haystack.
358+
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 'ucs2'), -1);
359+
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 'utf8'), -1);
360+
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 'binary'), -1);
361+
assert.strictEqual(b.lastIndexOf(Buffer.from('aaaaaaaaaaaaaaa')), -1);
362+
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 2, 'ucs2'), -1);
363+
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 3, 'utf8'), -1);
364+
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 5, 'binary'), -1);
365+
assert.strictEqual(b.lastIndexOf(Buffer.from('aaaaaaaaaaaaaaa'), 7), -1);
366+
367+
// 你好 expands to a total of 6 bytes using UTF-8 and 4 bytes using UTF-16
368+
assert.strictEqual(buf_bc.lastIndexOf('你好', 'ucs2'), -1);
369+
assert.strictEqual(buf_bc.lastIndexOf('你好', 'utf8'), -1);
370+
assert.strictEqual(buf_bc.lastIndexOf('你好', 'binary'), -1);
371+
assert.strictEqual(buf_bc.lastIndexOf(Buffer.from('你好')), -1);
372+
assert.strictEqual(buf_bc.lastIndexOf('你好', 2, 'ucs2'), -1);
373+
assert.strictEqual(buf_bc.lastIndexOf('你好', 3, 'utf8'), -1);
374+
assert.strictEqual(buf_bc.lastIndexOf('你好', 5, 'binary'), -1);
375+
assert.strictEqual(buf_bc.lastIndexOf(Buffer.from('你好'), 7), -1);
376+
357377
// Test lastIndexOf on a longer buffer:
358378
var bufferString = new Buffer('a man a plan a canal panama');
359379
assert.equal(15, bufferString.lastIndexOf('canal'));
360380
assert.equal(21, bufferString.lastIndexOf('panama'));
361381
assert.equal(0, bufferString.lastIndexOf('a man a plan a canal panama'));
362382
assert.equal(-1, bufferString.lastIndexOf('a man a plan a canal mexico'));
383+
assert.equal(-1, bufferString.lastIndexOf('a man a plan a canal mexico city'));
384+
assert.equal(-1, bufferString.lastIndexOf(Buffer.from('a'.repeat(1000))));
385+
assert.equal(0, bufferString.lastIndexOf('a man a plan', 4));
363386
assert.equal(13, bufferString.lastIndexOf('a '));
364387
assert.equal(13, bufferString.lastIndexOf('a ', 13));
365388
assert.equal(6, bufferString.lastIndexOf('a ', 12));

0 commit comments

Comments
 (0)