diff options
author | ZYSzys <17367077526@163.com> | 2019-02-17 16:59:10 +0300 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2019-02-22 00:08:53 +0300 |
commit | 6fb7baf935442a1ceddcd0a585892456438f95aa (patch) | |
tree | 0cacb7315eeda5389a6a4005064c183b17a82c5a | |
parent | dbfe14c8092ba914afc22d4ed8b0e1937c27ac25 (diff) |
buffer: harden validation of buffer allocation size
This makes using `NaN` as the buffer size throw an error.
Fixes: https://github.com/nodejs/node/issues/26151
PR-URL: https://github.com/nodejs/node/pull/26162
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
-rw-r--r-- | lib/buffer.js | 9 | ||||
-rw-r--r-- | test/parallel/test-buffer-alloc.js | 1 | ||||
-rw-r--r-- | test/parallel/test-buffer-no-negative-allocation.js | 6 | ||||
-rw-r--r-- | test/parallel/test-buffer-slow.js | 30 |
4 files changed, 20 insertions, 26 deletions
diff --git a/lib/buffer.js b/lib/buffer.js index 995f5096239..8b7aba1aaf0 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -256,7 +256,7 @@ function assertSize(size) { if (typeof size !== 'number') { err = new ERR_INVALID_ARG_TYPE('size', 'number', size); - } else if (size < 0 || size > kMaxLength) { + } else if (!(size >= 0 && size <= kMaxLength)) { err = new ERR_INVALID_OPT_VALUE.RangeError('size', size); } @@ -458,8 +458,11 @@ Buffer.concat = function concat(list, length) { if (length === undefined) { length = 0; - for (i = 0; i < list.length; i++) - length += list[i].length; + for (i = 0; i < list.length; i++) { + if (list[i].length) { + length += list[i].length; + } + } } else { length = length >>> 0; } diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index 8257a221c98..2b8e9c5f225 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -764,7 +764,6 @@ assert.strictEqual(x.inspect(), '<Buffer 81 a3 66 6f 6f a3 62 61 72>'); Buffer.allocUnsafe(3.3).fill().toString(); // throws bad argument error in commit 43cb4ec Buffer.alloc(3.3).fill().toString(); -assert.strictEqual(Buffer.allocUnsafe(NaN).length, 0); assert.strictEqual(Buffer.allocUnsafe(3.3).length, 3); assert.strictEqual(Buffer.from({ length: 3.3 }).length, 3); assert.strictEqual(Buffer.from({ length: 'BAM' }).length, 0); diff --git a/test/parallel/test-buffer-no-negative-allocation.js b/test/parallel/test-buffer-no-negative-allocation.js index b34477aa8c4..3a4958c8f62 100644 --- a/test/parallel/test-buffer-no-negative-allocation.js +++ b/test/parallel/test-buffer-no-negative-allocation.js @@ -7,22 +7,26 @@ const msg = common.expectsError({ code: 'ERR_INVALID_OPT_VALUE', type: RangeError, message: /^The value "[^"]*" is invalid for option "size"$/ -}, 12); +}, 16); // Test that negative Buffer length inputs throw errors. assert.throws(() => Buffer(-Buffer.poolSize), msg); assert.throws(() => Buffer(-100), msg); assert.throws(() => Buffer(-1), msg); +assert.throws(() => Buffer(NaN), msg); assert.throws(() => Buffer.alloc(-Buffer.poolSize), msg); assert.throws(() => Buffer.alloc(-100), msg); assert.throws(() => Buffer.alloc(-1), msg); +assert.throws(() => Buffer.alloc(NaN), msg); assert.throws(() => Buffer.allocUnsafe(-Buffer.poolSize), msg); assert.throws(() => Buffer.allocUnsafe(-100), msg); assert.throws(() => Buffer.allocUnsafe(-1), msg); +assert.throws(() => Buffer.allocUnsafe(NaN), msg); assert.throws(() => Buffer.allocUnsafeSlow(-Buffer.poolSize), msg); assert.throws(() => Buffer.allocUnsafeSlow(-100), msg); assert.throws(() => Buffer.allocUnsafeSlow(-1), msg); +assert.throws(() => Buffer.allocUnsafeSlow(NaN), msg); diff --git a/test/parallel/test-buffer-slow.js b/test/parallel/test-buffer-slow.js index cfea22b02cf..99362add3bb 100644 --- a/test/parallel/test-buffer-slow.js +++ b/test/parallel/test-buffer-slow.js @@ -43,29 +43,17 @@ try { assert.strictEqual(SlowBuffer('6').length, 6); assert.strictEqual(SlowBuffer(true).length, 1); -// Should create zero-length buffer if parameter is not a number -assert.strictEqual(SlowBuffer().length, 0); -assert.strictEqual(SlowBuffer(NaN).length, 0); -assert.strictEqual(SlowBuffer({}).length, 0); -assert.strictEqual(SlowBuffer('string').length, 0); - // should throw with invalid length const bufferMaxSizeMsg = common.expectsError({ code: 'ERR_INVALID_OPT_VALUE', type: RangeError, message: /^The value "[^"]*" is invalid for option "size"$/ -}, 2); -assert.throws(function() { - SlowBuffer(Infinity); -}, bufferMaxSizeMsg); -common.expectsError(function() { - SlowBuffer(-1); -}, { - code: 'ERR_INVALID_OPT_VALUE', - type: RangeError, - message: 'The value "-1" is invalid for option "size"' -}); - -assert.throws(function() { - SlowBuffer(buffer.kMaxLength + 1); -}, bufferMaxSizeMsg); +}, 7); + +assert.throws(() => SlowBuffer(), bufferMaxSizeMsg); +assert.throws(() => SlowBuffer(NaN), bufferMaxSizeMsg); +assert.throws(() => SlowBuffer({}), bufferMaxSizeMsg); +assert.throws(() => SlowBuffer('string'), bufferMaxSizeMsg); +assert.throws(() => SlowBuffer(Infinity), bufferMaxSizeMsg); +assert.throws(() => SlowBuffer(-1), bufferMaxSizeMsg); +assert.throws(() => SlowBuffer(buffer.kMaxLength + 1), bufferMaxSizeMsg); |