Skip to content
Permalink
Browse files

buffer: safeguard against accidental kNoZeroFill

This makes sure that `kNoZeroFill` flag is not accidentally set by
moving the all the flag operations directly inside `createBuffer()`.
It safeguards against logical errors like
#6006.

This also ensures that `kNoZeroFill` flag is always restored to 0 using
a try-finally block, as it could be not restored to 0 in cases of failed
or zero-size `Uint8Array` allocation.
It safeguards against errors like
#2930.
It also makes the `size > 0` check not needed there.

PR-URL: nodejs-private/node-private#30
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information...
ChALkeR authored and jasnell committed Apr 25, 2016
1 parent 2011f2c commit dd67608bfdb3bef6e06a1965ecbfa51658c61b1b
Showing with 15 additions and 20 deletions.
  1. +15 −20 lib/buffer.js
@@ -62,17 +62,20 @@ Buffer.prototype.swap32 = function swap32() {
const flags = bindingObj.flags;
const kNoZeroFill = 0;

function createBuffer(size) {
const ui8 = new Uint8Array(size);
Object.setPrototypeOf(ui8, Buffer.prototype);
return ui8;
function createBuffer(size, noZeroFill) {
flags[kNoZeroFill] = noZeroFill ? 1 : 0;
try {
const ui8 = new Uint8Array(size);
Object.setPrototypeOf(ui8, Buffer.prototype);
return ui8;
} finally {
flags[kNoZeroFill] = 0;
}
}

function createPool() {
poolSize = Buffer.poolSize;
if (poolSize > 0)
flags[kNoZeroFill] = 1;
allocPool = createBuffer(poolSize);
allocPool = createBuffer(poolSize, true);
poolOffset = 0;
}
createPool();
@@ -154,15 +157,13 @@ Buffer.alloc = function(size, fill, encoding) {
return createBuffer(size);
if (fill !== undefined) {
// Since we are filling anyway, don't zero fill initially.
flags[kNoZeroFill] = 1;
// Only pay attention to encoding if it's a string. This
// prevents accidentally sending in a number that would
// be interpretted as a start offset.
return typeof encoding === 'string' ?
createBuffer(size).fill(fill, encoding) :
createBuffer(size).fill(fill);
createBuffer(size, true).fill(fill, encoding) :
createBuffer(size, true).fill(fill);
}
flags[kNoZeroFill] = 0;
return createBuffer(size);
};

@@ -182,19 +183,15 @@ Buffer.allocUnsafe = function(size) {
**/
Buffer.allocUnsafeSlow = function(size) {
assertSize(size);
if (size > 0)
flags[kNoZeroFill] = 1;
return createBuffer(size);
return createBuffer(size, true);
};

// If --zero-fill-buffers command line argument is set, a zero-filled
// buffer is returned.
function SlowBuffer(length) {
if (+length != length)
length = 0;
if (length > 0)
flags[kNoZeroFill] = 1;
return createBuffer(+length);
return createBuffer(+length, true);
}

Object.setPrototypeOf(SlowBuffer.prototype, Uint8Array.prototype);
@@ -216,9 +213,7 @@ function allocate(size) {
// Even though this is checked above, the conditional is a safety net and
// sanity check to prevent any subsequent typed array allocation from not
// being zero filled.
if (size > 0)
flags[kNoZeroFill] = 1;
return createBuffer(size);
return createBuffer(size, true);
}
}

0 comments on commit dd67608

Please sign in to comment.
You can’t perform that action at this time.