1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

http: guard against response splitting in trailers

Browse Source 
Commit 3c293ba ("http: protect against response splitting attacks")
filters out newline characters from HTTP headers but forgot to apply
the same logic to trailing HTTP headers, i.e., headers that come after
the response body.  This commit rectifies that.

The expected security impact is low because approximately no one uses
trailing headers.  Some HTTP clients can't even parse them.

PR-URL: https://github.com/nodejs/node/pull/2945
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Rod Vagg <r@va.gg>
This commit is contained in:
Ben Noordhuis 2015-09-18 05:06:35 +02:00
parent a9ad31fb3d
commit e68a119c18
2 changed files with 22 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
lib/_http_outgoing.js
View File

@ -295,11 +295,7 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
};
function storeHeader(self, state, field, value) {
// Protect against response splitting. The if statement is there to
// minimize the performance impact in the common case.
if (/[\r\n]/.test(value))
value = value.replace(/[\r\n]+[ \t]*/g, '');
value = escapeHeaderValue(value);
state.messageHeader += field + ': ' + value + CRLF;
if (connectionExpression.test(field)) {
@ -481,6 +477,13 @@ function connectionCorkNT(conn) {
}
function escapeHeaderValue(value) {
// Protect against response splitting. The regex test is there to
// minimize the performance impact in the common case.
return /[\r\n]/.test(value) ? value.replace(/[\r\n]+[ \t]*/g, '') : value;
}
OutgoingMessage.prototype.addTrailers = function(headers) {
this._trailer = '';
var keys = Object.keys(headers);
@ -496,7 +499,7 @@ OutgoingMessage.prototype.addTrailers = function(headers) {
value = headers[key];
}
this._trailer += field + ': ' + value + CRLF;
this._trailer += field + ': ' + escapeHeaderValue(value) + CRLF;
}
};

16
test/parallel/test-http-header-response-splitting.js
View File

@ -4,7 +4,7 @@ var common = require('../common'),
http = require('http');
var testIndex = 0;
const testCount = 4 * 6;
const testCount = 2 * 4 * 6;
const responseBody = 'Hi mars!';
var server = http.createServer(function(req, res) {
@ -29,9 +29,15 @@ var server = http.createServer(function(req, res) {
default:
assert.fail('unreachable');
}
res.end(responseBody);
res.write(responseBody);
if (testIndex % 8 < 4) {
res.addTrailers({ ta: header, tb: header });
} else {
res.addTrailers([['ta', header], ['tb', header]]);
}
res.end();
}
switch ((testIndex / 4) | 0) {
switch ((testIndex / 8) | 0) {
case 0:
reply('foo \r\ninvalid: bar');
break;
@ -70,6 +76,10 @@ server.listen(common.PORT, common.mustCall(function() {
res.on('data', function(s) { data += s; });
res.on('end', common.mustCall(function() {
assert.equal(data, responseBody);
assert.strictEqual(res.trailers.ta, 'foo invalid: bar');
assert.strictEqual(res.trailers.tb, 'foo invalid: bar');
assert.strictEqual(res.trailers.foo, undefined);
assert.strictEqual(res.trailers.invalid, undefined);
}));
res.resume();
}));