From 186035243fad247e3955fa0c202987cae99e82db Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 21 Aug 2018 17:26:51 +0200 Subject: [PATCH] deps,http: http_parser set max header size to 8KB CVE-2018-12121 PR-URL: https://github.com/nodejs-private/node-private/pull/143 Ref: https://github.com/nodejs-private/security/issues/139 Ref: https://github.com/nodejs-private/http-parser-private/pull/2 Reviewed-By: Anatoli Papirovski Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Rod Vagg Reviewed-By: Anna Henningsen --- deps/http_parser/http_parser.gyp | 4 +- test/parallel/test-http-max-headers-count.js | 6 +- test/parallel/test-https-max-headers-count.js | 6 +- test/sequential/test-http-max-http-headers.js | 154 ++++++++++++++++++ 4 files changed, 162 insertions(+), 8 deletions(-) create mode 100644 test/sequential/test-http-max-http-headers.js diff --git a/deps/http_parser/http_parser.gyp b/deps/http_parser/http_parser.gyp index ef34ecaeaeab45..4364f73d1f4548 100644 --- a/deps/http_parser/http_parser.gyp +++ b/deps/http_parser/http_parser.gyp @@ -56,7 +56,7 @@ 'defines': [ 'HTTP_PARSER_STRICT=0' ], 'include_dirs': [ '.' ], }, - 'defines': [ 'HTTP_PARSER_STRICT=0' ], + 'defines': [ 'HTTP_MAX_HEADER_SIZE=8192', 'HTTP_PARSER_STRICT=0' ], 'sources': [ './http_parser.c', ], 'conditions': [ ['OS=="win"', { @@ -79,7 +79,7 @@ 'defines': [ 'HTTP_PARSER_STRICT=1' ], 'include_dirs': [ '.' ], }, - 'defines': [ 'HTTP_PARSER_STRICT=1' ], + 'defines': [ 'HTTP_MAX_HEADER_SIZE=8192', 'HTTP_PARSER_STRICT=1' ], 'sources': [ './http_parser.c', ], 'conditions': [ ['OS=="win"', { diff --git a/test/parallel/test-http-max-headers-count.js b/test/parallel/test-http-max-headers-count.js index 05f4f774c2c929..9fcfe316e392ff 100644 --- a/test/parallel/test-http-max-headers-count.js +++ b/test/parallel/test-http-max-headers-count.js @@ -28,14 +28,14 @@ let requests = 0; let responses = 0; const headers = {}; -const N = 2000; +const N = 100; for (let i = 0; i < N; ++i) { headers[`key${i}`] = i; } const maxAndExpected = [ // for server [50, 50], - [1500, 1500], + [1500, 102], [0, N + 2] // Host and Connection ]; let max = maxAndExpected[requests][0]; @@ -56,7 +56,7 @@ server.maxHeadersCount = max; server.listen(0, function() { const maxAndExpected = [ // for client [20, 20], - [1200, 1200], + [1200, 103], [0, N + 3] // Connection, Date and Transfer-Encoding ]; doRequest(); diff --git a/test/parallel/test-https-max-headers-count.js b/test/parallel/test-https-max-headers-count.js index 8c099d1e5fb841..12aaaa9cd3ad84 100644 --- a/test/parallel/test-https-max-headers-count.js +++ b/test/parallel/test-https-max-headers-count.js @@ -17,14 +17,14 @@ let requests = 0; let responses = 0; const headers = {}; -const N = 2000; +const N = 100; for (let i = 0; i < N; ++i) { headers[`key${i}`] = i; } const maxAndExpected = [ // for server [50, 50], - [1500, 1500], + [1500, 102], [0, N + 2] // Host and Connection ]; let max = maxAndExpected[requests][0]; @@ -45,7 +45,7 @@ server.maxHeadersCount = max; server.listen(0, common.mustCall(() => { const maxAndExpected = [ // for client [20, 20], - [1200, 1200], + [1200, 103], [0, N + 3] // Connection, Date and Transfer-Encoding ]; const doRequest = common.mustCall(() => { diff --git a/test/sequential/test-http-max-http-headers.js b/test/sequential/test-http-max-http-headers.js new file mode 100644 index 00000000000000..155b75fb076ae1 --- /dev/null +++ b/test/sequential/test-http-max-http-headers.js @@ -0,0 +1,154 @@ +'use strict'; + +const assert = require('assert'); +const common = require('../common'); +const http = require('http'); +const net = require('net'); +const MAX = 8 * 1024; // 8KB + +// Verify that we cannot receive more than 8KB of headers. + +function once(cb) { + let called = false; + return () => { + if (!called) { + called = true; + cb(); + } + }; +} + +function finished(client, callback) { + 'abort error end'.split(' ').forEach((e) => { + client.on(e, once(() => setImmediate(callback))); + }); +} + +function fillHeaders(headers, currentSize, valid = false) { + // Generate valid headers + if (valid) { + // TODO(mcollina): understand why -9 is needed instead of -1 + headers = headers.slice(0, -9); + } + return headers + '\r\n\r\n'; +} + +const timeout = common.platformTimeout(10); + +function writeHeaders(socket, headers) { + const array = []; + + // this is off from 1024 so that \r\n does not get split + const chunkSize = 1000; + let last = 0; + + for (let i = 0; i < headers.length / chunkSize; i++) { + const current = (i + 1) * chunkSize; + array.push(headers.slice(last, current)); + last = current; + } + + // safety check we are chunking correctly + assert.strictEqual(array.join(''), headers); + + next(); + + function next() { + if (socket.write(array.shift())) { + if (array.length === 0) { + socket.end(); + } else { + setTimeout(next, timeout); + } + } else { + socket.once('drain', next); + } + } +} + +function test1() { + let headers = + 'HTTP/1.1 200 OK\r\n' + + 'Content-Length: 0\r\n' + + 'X-CRASH: '; + + // OK, Content-Length, 0, X-CRASH, aaa... + const currentSize = 2 + 14 + 1 + 7; + headers = fillHeaders(headers, currentSize); + + const server = net.createServer((sock) => { + sock.once('data', (chunk) => { + writeHeaders(sock, headers); + sock.resume(); + }); + }); + + server.listen(0, common.mustCall(() => { + const port = server.address().port; + const client = http.get({ port: port }, common.mustNotCall(() => {})); + + client.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'HPE_HEADER_OVERFLOW'); + server.close(); + setImmediate(test2); + })); + })); +} + +const test2 = common.mustCall(() => { + let headers = + 'GET / HTTP/1.1\r\n' + + 'Host: localhost\r\n' + + 'Agent: node\r\n' + + 'X-CRASH: '; + + // /, Host, localhost, Agent, node, X-CRASH, a... + const currentSize = 1 + 4 + 9 + 5 + 4 + 7; + headers = fillHeaders(headers, currentSize); + + const server = http.createServer(common.mustNotCall()); + + server.on('clientError', common.mustCall((err) => { + assert.strictEqual(err.code, 'HPE_HEADER_OVERFLOW'); + })); + + server.listen(0, common.mustCall(() => { + const client = net.connect(server.address().port); + client.on('connect', () => { + writeHeaders(client, headers); + client.resume(); + }); + + finished(client, common.mustCall((err) => { + server.close(); + setImmediate(test3); + })); + })); +}); + +const test3 = common.mustCall(() => { + let headers = + 'GET / HTTP/1.1\r\n' + + 'Host: localhost\r\n' + + 'Agent: node\r\n' + + 'X-CRASH: '; + + // /, Host, localhost, Agent, node, X-CRASH, a... + const currentSize = 1 + 4 + 9 + 5 + 4 + 7; + headers = fillHeaders(headers, currentSize, true); + + const server = http.createServer(common.mustCall((req, res) => { + res.end('hello world'); + setImmediate(server.close.bind(server)); + })); + + server.listen(0, common.mustCall(() => { + const client = net.connect(server.address().port); + client.on('connect', () => { + writeHeaders(client, headers); + client.resume(); + }); + })); +}); + +test1();