Skip to content

Commit

Permalink
buffer: zero-fill uninitialized bytes in .concat()
Browse files Browse the repository at this point in the history
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/64
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
ChALkeR authored and evanlucas committed Sep 27, 2016
1 parent 743f0c9 commit 8fb8c46
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
8 changes: 8 additions & 0 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,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;
};

Expand Down
24 changes: 23 additions & 1 deletion test/parallel/test-buffer-concat.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');

const zero = [];
Expand Down Expand Up @@ -38,3 +38,25 @@ function assertWrongList(value) {
err.message === '"list" argument must be an Array of Buffers';
});
}

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)]));

0 comments on commit 8fb8c46

Please sign in to comment.