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
https://github.com/nodejs/node/issues/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
https://github.com/nodejs/node/issues/2930.
It also makes the `size > 0` check not needed there.

PR-URL: https://github.com/nodejs/node-private/pull/30
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

1 files changed, 12 insertions, 14 deletions

diff --git a/lib/buffer.js b/lib/buffer.js
index a24bdd0e006..eff91be2882 100644
--- a/lib/buffer.js
+++ b/lib/buffer.js
@@ -19,17 +19,20 @@ binding.setupBufferJS(Buffer.prototype, bindingObj);
 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();
@@ -65,13 +68,10 @@ function Buffer(arg, encoding) {
 Object.setPrototypeOf(Buffer.prototype, Uint8Array.prototype);
 Object.setPrototypeOf(Buffer, Uint8Array);
 
-
 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);
@@ -93,9 +93,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);
   }
 }