Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

http: protect against response splitting attacks #4292

Closed
wants to merge 1 commit into from

4 participants

@piscisaureus

Ref: #4290

@piscisaureus

cc @isaacs @TooTallNate @bnoordhuis @indutny

Ben doesn't want this and I want it very much.

@isaacs
Owner

We do this already for request headers, it seems. I think it's a good idea. Does it impact performance to do this test?

@koichik
Owner

Refs #2602.

@miksago miksago commented on the diff
lib/http.js
@@ -551,6 +551,11 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
var self = this;
function store(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, '');
+
@miksago
miksago added a note

Performance wise, it's probably a good idea to catch these regex's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@isaacs
Owner

Landed on 3c293ba.

@isaacs isaacs closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 19, 2012
  1. @piscisaureus
This page is out of date. Refresh to see the latest.
Showing with 69 additions and 0 deletions.
  1. +5 −0 lib/http.js
  2. +64 −0 test/simple/test-http-header-response-splitting.js
View
5 lib/http.js
@@ -551,6 +551,11 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
var self = this;
function store(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, '');
+
@miksago
miksago added a note

Performance wise, it's probably a good idea to catch these regex's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
messageHeader += field + ': ' + value + CRLF;
if (connectionExpression.test(field)) {
View
64 test/simple/test-http-header-response-splitting.js
@@ -0,0 +1,64 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common'),
+ assert = require('assert'),
+ http = require('http');
+
+var testIndex = 0,
+ responses = 0;
+
+var server = http.createServer(function(req, res) {
+ switch (testIndex++) {
+ case 0:
+ res.writeHead(200, { test: 'foo \r\ninvalid: bar' });
+ break;
+ case 1:
+ res.writeHead(200, { test: 'foo \ninvalid: bar' });
+ break;
+ case 2:
+ res.writeHead(200, { test: 'foo \rinvalid: bar' });
+ break;
+ case 3:
+ res.writeHead(200, { test: 'foo \n\n\ninvalid: bar' });
+ break;
+ case 4:
+ res.writeHead(200, { test: 'foo \r\n \r\n \r\ninvalid: bar' });
+ server.close();
+ break;
+ default:
+ assert(false);
+ }
+ res.end('Hi mars!');
+});
+server.listen(common.PORT);
+
+for (var i = 0; i < 5; i++) {
+ var req = http.get({ port: common.PORT, path: '/' }, function(res) {
+ assert.strictEqual(res.headers.test, 'foo invalid: bar');
+ assert.strictEqual(res.headers.invalid, undefined);
+ responses++;
+ });
+}
+
+process.on('exit', function() {
+ assert.strictEqual(responses, 5);
+});
Something went wrong with that request. Please try again.