Permalink
Browse files

buffer: throw on negative .allocUnsafe() argument

Add a check for `size < 0` to `assertSize()`, as passing a negative
value almost certainly indicates a programming error.

This also lines up the behaviour of `.allocUnsafe()` with the ones
of `.alloc()` and `.allocUnsafeSlow()` (which previously threw errors
from the Uint8Array constructor).

Notably, this also affects `Buffer()` calls with negative arguments.

PR-URL: #7079
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
  • Loading branch information...
addaleax committed May 31, 2016
1 parent be73480 commit 8f90dcc1b8e4ac3d8597ea2ee3927f325cc980d3
Showing with 38 additions and 8 deletions.
  1. +8 −2 lib/buffer.js
  2. +18 −1 test/parallel/test-buffer-alloc.js
  3. +12 −5 test/parallel/test-buffer.js
@@ -101,8 +101,14 @@ Buffer.from = function(value, encodingOrOffset, length) {
Object.setPrototypeOf(Buffer, Uint8Array);
function assertSize(size) {
if (typeof size !== 'number') {
const err = new TypeError('"size" argument must be a number');
let err = null;
if (typeof size !== 'number')
err = new TypeError('"size" argument must be a number');
else if (size < 0)
err = new RangeError('"size" argument must not be negative');
if (err) {
// The following hides the 'assertSize' method from the
// callstack. This is done simply to hide the internal
// details of the implementation from bleeding out to users.
@@ -981,7 +981,6 @@ assert.equal(0, Buffer.from('hello').slice(0, 0).length);
Buffer.allocUnsafe(3.3).fill().toString();
// throws bad argument error in commit 43cb4ec
Buffer.alloc(3.3).fill().toString();
assert.equal(Buffer.allocUnsafe(-1).length, 0);
assert.equal(Buffer.allocUnsafe(NaN).length, 0);
assert.equal(Buffer.allocUnsafe(3.3).length, 3);
assert.equal(Buffer.from({length: 3.3}).length, 3);
@@ -1479,3 +1478,21 @@ assert.equal(ubuf.buffer.byteLength, 10);
assert.doesNotThrow(() => {
Buffer.from(new ArrayBuffer());
});
assert.throws(() => Buffer.alloc(-Buffer.poolSize),
'"size" argument must not be negative');
assert.throws(() => Buffer.alloc(-100),
'"size" argument must not be negative');
assert.throws(() => Buffer.allocUnsafe(-Buffer.poolSize),
'"size" argument must not be negative');
assert.throws(() => Buffer.allocUnsafe(-100),
'"size" argument must not be negative');
assert.throws(() => Buffer.allocUnsafeSlow(-Buffer.poolSize),
'"size" argument must not be negative');
assert.throws(() => Buffer.allocUnsafeSlow(-100),
'"size" argument must not be negative');
assert.throws(() => Buffer.alloc({ valueOf: () => 1 }),
/"size" argument must be a number/);
assert.throws(() => Buffer.alloc({ valueOf: () => -1 }),
/"size" argument must be a number/);
@@ -1005,7 +1005,6 @@ assert.equal(0, Buffer('hello').slice(0, 0).length);
// Call .fill() first, stops valgrind warning about uninitialized memory reads.
Buffer(3.3).fill().toString(); // throws bad argument error in commit 43cb4ec
assert.equal(Buffer(-1).length, 0);
assert.equal(Buffer(NaN).length, 0);
assert.equal(Buffer(3.3).length, 3);
assert.equal(Buffer({length: 3.3}).length, 3);
@@ -1491,10 +1490,10 @@ assert.equal(SlowBuffer.prototype.offset, undefined);
{
// Test that large negative Buffer length inputs don't affect the pool offset.
assert.deepStrictEqual(Buffer(-Buffer.poolSize), Buffer.from(''));
assert.deepStrictEqual(Buffer(-100), Buffer.from(''));
assert.deepStrictEqual(Buffer.allocUnsafe(-Buffer.poolSize), Buffer.from(''));
assert.deepStrictEqual(Buffer.allocUnsafe(-100), Buffer.from(''));
// Use the fromArrayLike() variant here because it's more lenient
// about its input and passes the length directly to allocate().
assert.deepStrictEqual(Buffer({ length: -Buffer.poolSize }), Buffer.from(''));
assert.deepStrictEqual(Buffer({ length: -100 }), Buffer.from(''));
// Check pool offset after that by trying to write string into the pool.
assert.doesNotThrow(() => Buffer.from('abc'));
@@ -1522,3 +1521,11 @@ assert.equal(SlowBuffer.prototype.offset, undefined);
}
}
}
// Test that large negative Buffer length inputs throw errors.
assert.throws(() => Buffer(-Buffer.poolSize),
'"size" argument must not be negative');
assert.throws(() => Buffer(-100),
'"size" argument must not be negative');
assert.throws(() => Buffer(-1),
'"size" argument must not be negative');

0 comments on commit 8f90dcc

Please sign in to comment.