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: https://github.com/nodejs/node-private/pull/35
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
ChALkeR committed May 5, 2016
1 parent bec5d50 commit 35f06df
Showing 1 changed file with 14 additions and 17 deletions.
31 changes: 14 additions & 17 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 @@ -129,15 +132,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 @@ -154,9 +155,7 @@ Buffer.allocUnsafe = function(size) {
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 @@ -178,9 +177,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 35f06df

Please sign in to comment.