Skip to content

Commit

Permalink
src: avoid hanging on Buffer#fill 0-length input
Browse files Browse the repository at this point in the history
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 <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
ChALkeR authored and evanlucas committed Jun 12, 2018
1 parent 0b4a089 commit 555696d
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-buffer-alloc-is-filled.js
Original file line number Diff line number Diff line change
@@ -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);
}
}
16 changes: 16 additions & 0 deletions test/parallel/test-buffer-fill.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
{
Expand Down

0 comments on commit 555696d

Please sign in to comment.