From d866438ce1c33018ae537cce98dade81e502930c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=A1=D0=BA=D0=BE=D0=B2=D0=BE=D1=80=D0=BE=D0=B4=D0=B0=20?=
 =?UTF-8?q?=D0=9D=D0=B8=D0=BA=D0=B8=D1=82=D0=B0=20=D0=90=D0=BD=D0=B4=D1=80?=
 =?UTF-8?q?=D0=B5=D0=B5=D0=B2=D0=B8=D1=87?= <chalkerx@gmail.com>
Date: Mon, 25 Apr 2016 21:56:19 +0300
Subject: [PATCH] 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>
---
 lib/buffer.js | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/lib/buffer.js b/lib/buffer.js
index 9076ad24c0..515f841fd3 100644
--- a/lib/buffer.js
+++ b/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,9 +183,7 @@ 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
@@ -192,9 +191,7 @@ Buffer.allocUnsafeSlow = 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);
@@ -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);
   }
 }