Skip to content

Commit 3217e8e

Browse files
addaleaxMylesBorins
authored andcommitted
src: re-add Realloc() shrink after reading stream data
This would otherwise keep a lot of unused memory lying around, and in particular add up to a page per chunk of memory overhead for network reads, potentially opening a DoS vector if the resulting `Buffer` objects are kept around indefinitely (e.g. stored in a list and not concatenated until the socket finishes). This fixes CVE-2018-7164. Refs: https://github.com/nodejs-private/security/issues/186 Refs: 7c4b09b PR-URL: https://github.com/nodejs-private/node-private/pull/128 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
1 parent 785e5ba commit 3217e8e

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

src/stream_base.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -374,8 +374,9 @@ void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
374374
}
375375

376376
CHECK_LE(static_cast<size_t>(nread), buf.len);
377+
char* base = Realloc(buf.base, nread);
377378

378-
Local<Object> obj = Buffer::New(env, buf.base, nread).ToLocalChecked();
379+
Local<Object> obj = Buffer::New(env, base, nread).ToLocalChecked();
379380
stream->CallJSOnreadMethod(nread, obj);
380381
}
381382

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Flags: --expose-gc
2+
'use strict';
3+
4+
const common = require('../common');
5+
const assert = require('assert');
6+
const net = require('net');
7+
8+
// Tests that, when receiving small chunks, we do not keep the full length
9+
// of the original allocation for the libuv read call in memory.
10+
11+
let client;
12+
let baseRSS;
13+
const receivedChunks = [];
14+
const N = 250000;
15+
16+
const server = net.createServer(common.mustCall((socket) => {
17+
baseRSS = process.memoryUsage().rss;
18+
19+
socket.setNoDelay(true);
20+
socket.on('data', (chunk) => {
21+
receivedChunks.push(chunk);
22+
if (receivedChunks.length < N) {
23+
client.write('a');
24+
} else {
25+
client.end();
26+
server.close();
27+
}
28+
});
29+
})).listen(0, common.mustCall(() => {
30+
client = net.connect(server.address().port);
31+
client.setNoDelay(true);
32+
client.write('hello!');
33+
}));
34+
35+
process.on('exit', () => {
36+
global.gc();
37+
const bytesPerChunk =
38+
(process.memoryUsage().rss - baseRSS) / receivedChunks.length;
39+
// We should always have less than one page (usually ~ 4 kB) per chunk.
40+
assert(bytesPerChunk < 512, `measured ${bytesPerChunk} bytes per chunk`);
41+
});

0 commit comments

Comments
 (0)