Skip to content

Commit

Permalink
buffer: safeguard against accidental kNoZeroFill
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
  • Loading branch information
ChALkeR authored and jasnell committed Apr 26, 2016
1 parent 61be000 commit d866438
Showing 1 changed file with 15 additions and 20 deletions.
35 changes: 15 additions & 20 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
};

Expand All @@ -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);
Expand All @@ -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);
}
}

Expand Down

0 comments on commit d866438

Please sign in to comment.