Skip to content

Commit

Permalink
buffer: harden validation of buffer allocation size
Browse files Browse the repository at this point in the history
This makes using `NaN` as the buffer size throw an error.

Fixes: #26151

PR-URL: #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>
  • Loading branch information
ZYSzys authored and addaleax committed Feb 21, 2019
1 parent dbfe14c commit 6fb7baf
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 26 deletions.
9 changes: 6 additions & 3 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion test/parallel/test-buffer-no-negative-allocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
30 changes: 9 additions & 21 deletions test/parallel/test-buffer-slow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit 6fb7baf

Please sign in to comment.