Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request timeout if a single header exceeds the max-http-header-size limit #25858

Closed
natedanner opened this issue Jan 31, 2019 · 5 comments
Closed

Comments

@natedanner
Copy link

natedanner commented Jan 31, 2019

  • Version: 10.15.1
  • Platform: 18.2.0 Darwin Kernel Version 18.2.0: Thu Dec 20 20:46:53 PST 2018; root:xnu-4903.241.1~1/RELEASE_X86_64 x86_64
  • Subsystem: http

If a request header exceeds the max-http-header-size in Node 10.15.1 and 10.15.0 the request hangs leaving the socket open until the server times out. This seems like it could be a potential DoS attack vector. On Node 10.14.1 a 400 response is received as expected. The code below demonstrates the issue. If you run the code on 10.14.1 both requests will return immediately with a 400 as expected. On 10.15.1 the second request times out.

You can grab the test script from here also. https://github.com/natedanner/node-max-header-timeout/blob/master/server.js

const http = require('http');
const port = 3000;

const server = http.createServer((req, res) => {
	res.writeHead(200, {'Content-Type': 'text/plain; charset=UTF-8'});
	res.end('Hello World\n');
})

server.listen(port, () => {
	console.log(`Listening on ${port}`);

	// Headers that together exceed default 8k limit
	const defaultHeaders = {
		'FOO': '6'.repeat(8100),
		'BAR': '6'.repeat(8192)
	};
	
	function callServer(callerName, headers) {
		return http.get('http://localhost:3000', { headers }, (response) => {
			console.log(callerName, 'responded with:', response.statusCode);
		});
	}
	
	const excessHeadersRequest = callServer('EXCESS_HEADER', defaultHeaders);
	excessHeadersRequest.setTimeout(2000, () => {
		console.log('SHOULD NOT SEE THIS');
	});

	// If you exceed header limit on a single header by 1 then the request hangs open
	defaultHeaders['BAR'] += '6'
	const headerTimeoutRequest = callServer('EXCESS_HEADER_TIMEOUT', defaultHeaders);
	headerTimeoutRequest.setTimeout(2000, () => {
		console.log('EXCESS_HEADER_TIMEOUT CALL TIMED OUT!');
	});
});
@natedanner natedanner changed the title Request timeout on max-http-header-size if a single header exceeds the limit Request timeout if a single header exceeds the max-http-header-size limit Jan 31, 2019
@mscdex
Copy link
Contributor

mscdex commented Feb 1, 2019

FWIW this works on master, so there's just a commit/change that needs to be backported it seems.

@mscdex
Copy link
Contributor

mscdex commented Feb 1, 2019

Found the change, will create a PR.

mscdex added a commit to mscdex/io.js that referenced this issue Feb 1, 2019
BethGriggs pushed a commit that referenced this issue Feb 5, 2019
Refs: #24738
Fixes: #25858

PR-URL: #25863
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
rvagg pushed a commit that referenced this issue Feb 28, 2019
Refs: #24738
Fixes: #25858

PR-URL: #25863
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
mscdex added a commit to mscdex/io.js that referenced this issue Feb 28, 2019
mscdex added a commit to mscdex/io.js that referenced this issue Mar 9, 2019
BethGriggs pushed a commit that referenced this issue Mar 11, 2019
Refs: #24738
Fixes: #25858

PR-URL: #25939
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
@mmarchini
Copy link
Contributor

I believe this was fixed on 10.15.3 (although I haven't tested it). Can this issue be closed?

@mmarchini
Copy link
Contributor

Ok, so I tried on v10.14.0, v10.15.1 and v10.15.3, and v10.19.0 the issue seems resolved on 10.15.3 and onwards.

$ nvm use 10.14.0
Now using node v10.14.0 (npm v6.4.1)

$ node index.js
Listening on 3000
EXCESS_HEADER responded with: 400
EXCESS_HEADER_TIMEOUT responded with: 400
^C

$ nvm install 10.15.1
Now using node v10.15.1 (npm v6.4.1)

$ node index.js
Listening on 3000
EXCESS_HEADER responded with: 400
EXCESS_HEADER_TIMEOUT CALL TIMED OUT!
^C

$ nvm install 10.15.3
Now using node v10.15.3 (npm v6.4.1)

$ node index.js
Listening on 3000
EXCESS_HEADER responded with: 400
EXCESS_HEADER_TIMEOUT responded with: 400
^C

$ nvm install 10.19.0
Now using node v10.19.0 (npm v6.13.4)

$ node index.js
Listening on 3000
EXCESS_HEADER responded with: 400
EXCESS_HEADER_TIMEOUT responded with: 400

Based on that, I'm closing this issue. Feel free to reopen if I missed something though.

@thomas-zhou
Copy link

@mmarchini we encountered this issue in the latest node version 14.19.0, may I know whether this issue has been fixed in v14? From the PR I only see that it has been fixed in v6, v8 and v10. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants