From 93b10fbec247a16504fb130cd394cf7411a0717a 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?= Date: Fri, 16 Sep 2016 08:07:23 +0300 Subject: [PATCH] buffer: zero-fill uninitialized bytes in .concat() This makes sure that no uninitialized bytes are leaked when the specified `totalLength` input value is greater than the actual total length of the specified buffers array, e.g. in Buffer.concat([Buffer.alloc(0)], 100). PR-URL: https://github.com/nodejs/node-private/pull/65 Reviewed-By: Anna Henningsen Reviewed-By: Rod Vagg --- lib/buffer.js | 8 ++++++++ test/parallel/test-buffer-concat.js | 24 +++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/buffer.js b/lib/buffer.js index a78ada9f8e72aa..8efb4cd789492b 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -291,6 +291,14 @@ Buffer.concat = function(list, length) { pos += buf.length; } + // Note: `length` is always equal to `buffer.length` at this point + if (pos < length) { + // Zero-fill the remaining bytes if the specified `length` was more than + // the actual total length, i.e. if we have some remaining allocated bytes + // there were not initialized. + buffer.fill(0, pos, length); + } + return buffer; }; diff --git a/test/parallel/test-buffer-concat.js b/test/parallel/test-buffer-concat.js index 07f763a76dc419..d00ce4a1b0bb1b 100644 --- a/test/parallel/test-buffer-concat.js +++ b/test/parallel/test-buffer-concat.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +var common = require('../common'); var assert = require('assert'); var zero = []; @@ -24,4 +24,26 @@ assert.throws(function() { Buffer.concat([42]); }, TypeError); +const random10 = common.hasCrypto + ? require('crypto').randomBytes(10) + : Buffer.alloc(10, 1); +const empty = Buffer.alloc(0); + +assert.notDeepStrictEqual(random10, empty); +assert.notDeepStrictEqual(random10, Buffer.alloc(10)); + +assert.deepStrictEqual(Buffer.concat([], 100), empty); +assert.deepStrictEqual(Buffer.concat([random10], 0), empty); +assert.deepStrictEqual(Buffer.concat([random10], 10), random10); +assert.deepStrictEqual(Buffer.concat([random10, random10], 10), random10); +assert.deepStrictEqual(Buffer.concat([empty, random10]), random10); +assert.deepStrictEqual(Buffer.concat([random10, empty, empty]), random10); + +// The tail should be zero-filled +assert.deepStrictEqual(Buffer.concat([empty], 100), Buffer.alloc(100)); +assert.deepStrictEqual(Buffer.concat([empty], 4096), Buffer.alloc(4096)); +assert.deepStrictEqual( + Buffer.concat([random10], 40), + Buffer.concat([random10, Buffer.alloc(30)])); + console.log('ok');