From 55b975d402a796ab5dcd8efe903646f58438054e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 2 May 2016 03:52:46 +0200 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/6510 PR-URL: https://github.com/nodejs/node/pull/6511 Reviewed-By: James M Snell Reviewed-By: Trevor Norris --- src/node_buffer.cc | 6 ++++-- test/parallel/test-buffer-indexof.js | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 44020d91a1..bb77387e8f 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1011,7 +1011,8 @@ void IndexOfString(const FunctionCallbackInfo& args) { } size_t offset = static_cast(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); } @@ -1113,7 +1114,8 @@ void IndexOfBuffer(const FunctionCallbackInfo& args) { } size_t offset = static_cast(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); } diff --git a/test/parallel/test-buffer-indexof.js b/test/parallel/test-buffer-indexof.js index 0cdce9cf25..205a8dfa6a 100644 --- a/test/parallel/test-buffer-indexof.js +++ b/test/parallel/test-buffer-indexof.js @@ -354,12 +354,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));