src: avoid hanging on Buffer#fill 0-length input

Previously, zero-length Buffers and TypedArrays passed as fillers hanged Buffer#fill and Buffer.from. This changes those cases when it hanged to a zero-fill instead, which should be backwards compatible. This fixes CVE-2018-7167. PR-URL: https://github.com/nodejs-private/node-private/pull/120 Fixes: https://github.com/nodejs-private/security/issues/193 Refs: https://github.com/nodejs-private/node-private/pull/118 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
nodejs · Jun 12, 2018 · 555696d · 555696d
1 parent 0b4a089
commit 555696d
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 0 deletions.
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -642,6 +642,12 @@ void Fill(const FunctionCallbackInfo<Value>& args) {
   size_t in_there = str_length;
   char* ptr = ts_obj_data + start + str_length;
 
+  if (in_there == 0) {
+    // Just use zero-fill if the input was empty
+    memset(ts_obj_data + start, 0, fill_length);
+    return;
+  }
+
   while (in_there < fill_length - in_there) {
     memcpy(ptr, ts_obj_data + start, in_there);
     ptr += in_there;

diff --git a/test/parallel/test-buffer-alloc-is-filled.js b/test/parallel/test-buffer-alloc-is-filled.js
@@ -0,0 +1,20 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+
+for (const fill of [
+  '',
+  [],
+  Buffer.from(''),
+  new Uint8Array(0),
+  { toString: () => '' },
+  { toString: () => '', length: 10 }
+]) {
+  for (let i = 0; i < 50; i++) {
+    const buf = Buffer.alloc(100, fill);
+    assert.strictEqual(buf.length, 100);
+    for (let n = 0; n < buf.length; n++)
+      assert.strictEqual(buf[n], 0);
+  }
+}
diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js
@@ -349,6 +349,22 @@ Buffer.alloc(8, '');
   assert.strictEqual(buf.toString(), 'էէէէէ');
 }
 
+{
+  for (const fill of [
+    '',
+    [],
+    Buffer.from(''),
+    new Uint8Array(0),
+    { toString: () => '' },
+    { toString: () => '', length: 10 }
+  ]) {
+    assert.deepStrictEqual(
+      Buffer.alloc(10, 'abc').fill(fill),
+      Buffer.alloc(10)
+    );
+  }
+}
+
 // Testing public API. Make sure "start" is properly checked, even if it's
 // magically mangled using Symbol.toPrimitive.
 {