Skip to content

Commit

Permalink
buffer: fix lastIndexOf crash for overlong needle
Browse files Browse the repository at this point in the history
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: #6510
PR-URL: #6511
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
addaleax authored and evanlucas committed May 17, 2016
1 parent 4401575 commit 6fc20c5
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/node_buffer.cc
Expand Up @@ -1009,7 +1009,8 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
}
size_t offset = static_cast<size_t>(opt_offset);
CHECK_LT(offset, haystack_length);
if (is_forward && needle_length + offset > haystack_length) {
if ((is_forward && needle_length + offset > haystack_length) ||
needle_length > haystack_length) {
return args.GetReturnValue().Set(-1);
}

Expand Down Expand Up @@ -1111,7 +1112,8 @@ void IndexOfBuffer(const FunctionCallbackInfo<Value>& args) {
}
size_t offset = static_cast<size_t>(opt_offset);
CHECK_LT(offset, haystack_length);
if (is_forward && needle_length + offset > haystack_length) {
if ((is_forward && needle_length + offset > haystack_length) ||
needle_length > haystack_length) {
return args.GetReturnValue().Set(-1);
}

Expand Down
23 changes: 23 additions & 0 deletions test/parallel/test-buffer-indexof.js
Expand Up @@ -345,12 +345,35 @@ assert.equal(b.lastIndexOf('b', {}), 1);
assert.equal(b.lastIndexOf('b', []), -1);
assert.equal(b.lastIndexOf('b', [2]), 1);

// Test needles longer than the haystack.
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 'ucs2'), -1);
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 'utf8'), -1);
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 'binary'), -1);
assert.strictEqual(b.lastIndexOf(Buffer.from('aaaaaaaaaaaaaaa')), -1);
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 2, 'ucs2'), -1);
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 3, 'utf8'), -1);
assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 5, 'binary'), -1);
assert.strictEqual(b.lastIndexOf(Buffer.from('aaaaaaaaaaaaaaa'), 7), -1);

// 你好 expands to a total of 6 bytes using UTF-8 and 4 bytes using UTF-16
assert.strictEqual(buf_bc.lastIndexOf('你好', 'ucs2'), -1);
assert.strictEqual(buf_bc.lastIndexOf('你好', 'utf8'), -1);
assert.strictEqual(buf_bc.lastIndexOf('你好', 'binary'), -1);
assert.strictEqual(buf_bc.lastIndexOf(Buffer.from('你好')), -1);
assert.strictEqual(buf_bc.lastIndexOf('你好', 2, 'ucs2'), -1);
assert.strictEqual(buf_bc.lastIndexOf('你好', 3, 'utf8'), -1);
assert.strictEqual(buf_bc.lastIndexOf('你好', 5, 'binary'), -1);
assert.strictEqual(buf_bc.lastIndexOf(Buffer.from('你好'), 7), -1);

// Test lastIndexOf on a longer buffer:
var bufferString = new Buffer('a man a plan a canal panama');
assert.equal(15, bufferString.lastIndexOf('canal'));
assert.equal(21, bufferString.lastIndexOf('panama'));
assert.equal(0, bufferString.lastIndexOf('a man a plan a canal panama'));
assert.equal(-1, bufferString.lastIndexOf('a man a plan a canal mexico'));
assert.equal(-1, bufferString.lastIndexOf('a man a plan a canal mexico city'));
assert.equal(-1, bufferString.lastIndexOf(Buffer.from('a'.repeat(1000))));
assert.equal(0, bufferString.lastIndexOf('a man a plan', 4));
assert.equal(13, bufferString.lastIndexOf('a '));
assert.equal(13, bufferString.lastIndexOf('a ', 13));
assert.equal(6, bufferString.lastIndexOf('a ', 12));
Expand Down

0 comments on commit 6fc20c5

Please sign in to comment.