Skip to content

Commit 555696d

Browse files
ChALkeRevanlucas
authored andcommitted
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]>
1 parent 0b4a089 commit 555696d

File tree

3 files changed

+42
-0
lines changed

3 files changed

+42
-0
lines changed

src/node_buffer.cc

+6
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,12 @@ void Fill(const FunctionCallbackInfo<Value>& args) {
642642
size_t in_there = str_length;
643643
char* ptr = ts_obj_data + start + str_length;
644644

645+
if (in_there == 0) {
646+
// Just use zero-fill if the input was empty
647+
memset(ts_obj_data + start, 0, fill_length);
648+
return;
649+
}
650+
645651
while (in_there < fill_length - in_there) {
646652
memcpy(ptr, ts_obj_data + start, in_there);
647653
ptr += in_there;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
'use strict';
2+
3+
require('../common');
4+
const assert = require('assert');
5+
6+
for (const fill of [
7+
'',
8+
[],
9+
Buffer.from(''),
10+
new Uint8Array(0),
11+
{ toString: () => '' },
12+
{ toString: () => '', length: 10 }
13+
]) {
14+
for (let i = 0; i < 50; i++) {
15+
const buf = Buffer.alloc(100, fill);
16+
assert.strictEqual(buf.length, 100);
17+
for (let n = 0; n < buf.length; n++)
18+
assert.strictEqual(buf[n], 0);
19+
}
20+
}

test/parallel/test-buffer-fill.js

+16
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,22 @@ Buffer.alloc(8, '');
349349
assert.strictEqual(buf.toString(), 'էէէէէ');
350350
}
351351

352+
{
353+
for (const fill of [
354+
'',
355+
[],
356+
Buffer.from(''),
357+
new Uint8Array(0),
358+
{ toString: () => '' },
359+
{ toString: () => '', length: 10 }
360+
]) {
361+
assert.deepStrictEqual(
362+
Buffer.alloc(10, 'abc').fill(fill),
363+
Buffer.alloc(10)
364+
);
365+
}
366+
}
367+
352368
// Testing public API. Make sure "start" is properly checked, even if it's
353369
// magically mangled using Symbol.toPrimitive.
354370
{

0 commit comments

Comments
 (0)