Skip to content

Commit 618eebd

Browse files
mcollinarvagg
authored andcommitted
http,https: protect against slow headers attack
CVE-2018-12122 An attacker can send a char/s within headers and exahust the resources (file descriptors) of a system even with a tight max header length protection. This PR destroys a socket if it has not received the headers in 40s. PR-URL: nodejs-private/node-private#152 Ref: nodejs-private/node-private#144 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 92231a5 commit 618eebd

7 files changed

+196
-11
lines changed

doc/api/http.md

+20
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,26 @@ for handling socket timeouts.
848848

849849
Returns `server`.
850850

851+
### server.headersTimeout
852+
<!-- YAML
853+
added: REPLACEME
854+
-->
855+
856+
* {number} **Default:** `40000`
857+
858+
Limit the amount of time the parser will wait to receive the complete HTTP
859+
headers.
860+
861+
In case of inactivity, the rules defined in [server.timeout][] apply. However,
862+
that inactivity based timeout would still allow the connection to be kept open
863+
if the headers are being sent very slowly (by default, up to a byte per 2
864+
minutes). In order to prevent this, whenever header data arrives an additional
865+
check is made that more than `server.headersTimeout` milliseconds has not
866+
passed since the connection was established. If the check fails, a `'timeout'`
867+
event is emitted on the server object, and (by default) the socket is destroyed.
868+
See [server.timeout][] for more information on how timeout behaviour can be
869+
customised.
870+
851871
### server.timeout
852872
<!-- YAML
853873
added: v0.9.12

doc/api/https.md

+7
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ added: v0.3.4
2323
This class is a subclass of `tls.Server` and emits events same as
2424
[`http.Server`][]. See [`http.Server`][] for more information.
2525

26+
### server.headersTimeout
27+
28+
- {number} **Default:** `40000`
29+
30+
See [`http.Server#headersTimeout`][].
31+
2632
### server.setTimeout([msecs][, callback])
2733
<!-- YAML
2834
added: v0.11.2
@@ -233,6 +239,7 @@ const req = https.request(options, (res) => {
233239
[`Buffer`]: buffer.html#buffer_buffer
234240
[`globalAgent`]: #https_https_globalagent
235241
[`http.Agent`]: http.html#http_class_http_agent
242+
[`http.Server#headersTimeout`]: http.html#http_server_headerstimeout
236243
[`http.close()`]: http.html#http_server_close_callback
237244
[`http.get()`]: http.html#http_http_get_options_callback
238245
[`http.listen()`]: http.html#http_server_listen_port_hostname_backlog_callback

lib/_http_outgoing.js

+23-10
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,33 @@ const automaticHeaders = {
3030
};
3131

3232

33-
var dateCache;
33+
var nowCache;
34+
var utcCache;
35+
36+
function nowDate() {
37+
if (!nowCache) cache();
38+
return nowCache;
39+
}
40+
3441
function utcDate() {
35-
if (!dateCache) {
36-
var d = new Date();
37-
dateCache = d.toUTCString();
38-
timers.enroll(utcDate, 1000 - d.getMilliseconds());
39-
timers._unrefActive(utcDate);
40-
}
41-
return dateCache;
42+
if (!utcCache) cache();
43+
return utcCache;
4244
}
43-
utcDate._onTimeout = function() {
44-
dateCache = undefined;
45+
46+
function cache() {
47+
const d = new Date();
48+
nowCache = d.valueOf();
49+
utcCache = d.toUTCString();
50+
timers.enroll(cache, 1000 - d.getMilliseconds());
51+
timers._unrefActive(cache);
52+
}
53+
54+
cache._onTimeout = function() {
55+
nowCache = undefined;
56+
utcCache = undefined;
4557
};
4658

59+
exports.nowDate = nowDate;
4760

4861
function OutgoingMessage() {
4962
Stream.call(this);

lib/_http_server.js

+19-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const continueExpression = common.continueExpression;
1313
const chunkExpression = common.chunkExpression;
1414
const httpSocketSetup = common.httpSocketSetup;
1515
const OutgoingMessage = require('_http_outgoing').OutgoingMessage;
16+
const nowDate = require('_http_outgoing').nowDate;
1617

1718
const STATUS_CODES = exports.STATUS_CODES = {
1819
100: 'Continue',
@@ -244,6 +245,7 @@ function Server(requestListener) {
244245
this.timeout = 2 * 60 * 1000;
245246

246247
this._pendingResponseData = 0;
248+
this.headersTimeout = 40 * 1000; // 40 seconds
247249
}
248250
util.inherits(Server, net.Server);
249251

@@ -317,6 +319,9 @@ function connectionListener(socket) {
317319
var parser = parsers.alloc();
318320
parser.reinitialize(HTTPParser.REQUEST);
319321
parser.socket = socket;
322+
323+
// We are starting to wait for our headers.
324+
parser.parsingHeadersStart = nowDate();
320325
socket.parser = parser;
321326
parser.incoming = null;
322327

@@ -374,6 +379,20 @@ function connectionListener(socket) {
374379
function onParserExecute(ret, d) {
375380
socket._unrefTimer();
376381
debug('SERVER socketOnParserExecute %d', ret);
382+
383+
var start = parser.parsingHeadersStart;
384+
385+
// If we have not parsed the headers, destroy the socket
386+
// after server.headersTimeout to protect from DoS attacks.
387+
// start === 0 means that we have parsed headers.
388+
if (start !== 0 && nowDate() - start > self.headersTimeout) {
389+
var serverTimeout = self.emit('timeout', socket);
390+
391+
if (!serverTimeout)
392+
socket.destroy();
393+
return;
394+
}
395+
377396
onParserExecuteCommon(ret, undefined);
378397
}
379398

@@ -442,7 +461,6 @@ function connectionListener(socket) {
442461
}
443462
}
444463

445-
446464
// The following callback is issued after the headers have been read on a
447465
// new message. In this callback we setup the response object and pass it
448466
// to the user.

lib/https.js

+2
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ function Server(opts, requestListener) {
3737
});
3838

3939
this.timeout = 2 * 60 * 1000;
40+
41+
this.headersTimeout = 40 * 1000; // 40 seconds
4042
}
4143
inherits(Server, tls.Server);
4244
exports.Server = Server;
+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const { createServer } = require('http');
6+
const { connect } = require('net');
7+
8+
// This test validates that the 'timeout' event fires
9+
// after server.headersTimeout.
10+
11+
const headers =
12+
'GET / HTTP/1.1\r\n' +
13+
'Host: localhost\r\n' +
14+
'Agent: node\r\n';
15+
16+
const server = createServer(common.mustNotCall());
17+
let sendCharEvery = 1000;
18+
19+
// 40 seconds is the default
20+
assert.strictEqual(server.headersTimeout, 40 * 1000);
21+
22+
// Pass a REAL env variable to shortening up the default
23+
// value which is 40s otherwise this is useful for manual
24+
// testing
25+
if (!process.env.REAL) {
26+
sendCharEvery = common.platformTimeout(10);
27+
server.headersTimeout = 2 * sendCharEvery;
28+
}
29+
30+
server.once('timeout', common.mustCall((socket) => {
31+
socket.destroy();
32+
}));
33+
34+
server.listen(0, common.mustCall(() => {
35+
const client = connect(server.address().port);
36+
client.write(headers);
37+
client.write('X-CRASH: ');
38+
39+
const interval = setInterval(() => {
40+
client.write('a');
41+
}, sendCharEvery);
42+
43+
client.resume();
44+
45+
const onClose = common.mustCall(() => {
46+
client.removeListener('close', onClose);
47+
client.removeListener('error', onClose);
48+
client.removeListener('end', onClose);
49+
clearInterval(interval);
50+
server.close();
51+
});
52+
53+
client.on('error', onClose);
54+
client.on('close', onClose);
55+
client.on('end', onClose);
56+
}));
+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const { readKey } = require('../common/fixtures');
5+
6+
if (!common.hasCrypto)
7+
common.skip('missing crypto');
8+
9+
const assert = require('assert');
10+
const { createServer } = require('https');
11+
const { connect } = require('tls');
12+
13+
// This test validates that the 'timeout' event fires
14+
// after server.headersTimeout.
15+
16+
const headers =
17+
'GET / HTTP/1.1\r\n' +
18+
'Host: localhost\r\n' +
19+
'Agent: node\r\n';
20+
21+
const server = createServer({
22+
key: readKey('agent1-key.pem'),
23+
cert: readKey('agent1-cert.pem'),
24+
ca: readKey('ca1-cert.pem'),
25+
}, common.mustNotCall());
26+
27+
let sendCharEvery = 1000;
28+
29+
// 40 seconds is the default
30+
assert.strictEqual(server.headersTimeout, 40 * 1000);
31+
32+
// pass a REAL env variable to shortening up the default
33+
// value which is 40s otherwise
34+
// this is useful for manual testing
35+
if (!process.env.REAL) {
36+
sendCharEvery = common.platformTimeout(10);
37+
server.headersTimeout = 2 * sendCharEvery;
38+
}
39+
40+
server.once('timeout', common.mustCall((socket) => {
41+
socket.destroy();
42+
}));
43+
44+
server.listen(0, common.mustCall(() => {
45+
const client = connect({
46+
port: server.address().port,
47+
rejectUnauthorized: false
48+
});
49+
client.write(headers);
50+
client.write('X-CRASH: ');
51+
52+
const interval = setInterval(() => {
53+
client.write('a');
54+
}, sendCharEvery);
55+
56+
client.resume();
57+
58+
const onClose = common.mustCall(() => {
59+
client.removeListener('close', onClose);
60+
client.removeListener('error', onClose);
61+
client.removeListener('end', onClose);
62+
clearInterval(interval);
63+
server.close();
64+
});
65+
66+
client.on('error', onClose);
67+
client.on('close', onClose);
68+
client.on('end', onClose);
69+
}));

0 commit comments

Comments
 (0)