http: strictly forbid invalid characters from headers

PR-URL: https://github.com/nodejs/node-private/pull/22

doc/api/http.markdown

@@ -278,6 +278,9 @@ should be used to determine the number of bytes in a given encoding.
 And Node does not check whether Content-Length and the length of the body
 which has been transmitted are equal or not.
 
+Attempting to set a header field name or value that contains invalid characters
+will result in a [`TypeError`][] being thrown.
+
 ### response.setTimeout(msecs, callback)
 
 * `msecs` {Number}
@@ -320,6 +323,9 @@ or
 
     response.setHeader("Set-Cookie", ["type=ninja", "language=javascript"]);
 
+Attempting to set a header field name or value that contains invalid characters
+will result in a [`TypeError`][] being thrown.
+
 ### response.headersSent
 
 Boolean (read-only). True if headers were sent, false otherwise.
@@ -394,6 +400,8 @@ emit trailers, with a list of the header fields in its value. E.g.,
     response.addTrailers({'Content-MD5': "7895bf4b8828b55ceaf47747b4bca667"});
     response.end();
 
+Attempting to set a header field name or value that contains invalid characters
+will result in a [`TypeError`][] being thrown.
 
 ### response.end([data], [encoding])
 

lib/http.js

@@ -30,6 +30,36 @@ var FreeList = require('freelist').FreeList;
 var HTTPParser = process.binding('http_parser').HTTPParser;
 var assert = require('assert').ok;
 
+var lenientHttpHeaders = !!process.REVERT_CVE_2016_2216;
+
+function escapeHeaderValue(value) {
+  if (!lenientHttpHeaders) return 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;
+}
+
+// Verifies that the given val is a valid HTTP token
+// per the rules defined in RFC 7230
+var token = /^[a-zA-Z0-9_!#$%&'*+.^`|~-]+$/;
+function _checkIsHttpToken(val) {
+  return typeof val === 'string' && token.test(val);
+}
+
+// True if val contains an invalid field-vchar
+//  field-value    = *( field-content / obs-fold )
+//  field-content  = field-vchar [ 1*( SP / HTAB ) field-vchar ]
+//  field-vchar    = VCHAR / obs-text
+function _checkInvalidHeaderChar(val) {
+  val = '' + val;
+  for (var i = 0; i < val.length; i++) {
+    var ch = val.charCodeAt(i);
+    if (ch === 9) continue;
+    if (ch <= 31 || ch > 255 || ch === 127) return true;
+  }
+  return false;
+}
+
 var debug;
 if (process.env.NODE_DEBUG && /http/.test(process.env.NODE_DEBUG)) {
   debug = function(x) { console.error('HTTP: %s', x); };
@@ -652,12 +682,16 @@ 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, '');
-
-  state.messageHeader += field + ': ' + value + CRLF;
+  if (!lenientHttpHeaders) {
+    if (!_checkIsHttpToken(field)) {
+      throw new TypeError(
+          'Header name must be a valid HTTP Token ["' + field + '"]');
+    }
+    if (_checkInvalidHeaderChar(value) === true) {
+      throw new TypeError('The header content contains invalid characters');
+    }
+  }
+  state.messageHeader += field + ': ' + escapeHeaderValue(value) + CRLF;
 
   if (connectionExpression.test(field)) {
     state.sentConnectionHeader = true;
@@ -690,6 +724,16 @@ OutgoingMessage.prototype.setHeader = function(name, value) {
     throw new Error('Can\'t set headers after they are sent.');
   }
 
+  if (!lenientHttpHeaders) {
+    if (!_checkIsHttpToken(name)) {
+      throw new TypeError(
+          'Trailer name must be a valid HTTP Token ["' + name + '"]');
+    }
+    if (_checkInvalidHeaderChar(value) === true) {
+      throw new TypeError('The header content contains invalid characters');
+    }
+  }
+
   var key = name.toLowerCase();
   this._headers = this._headers || {};
   this._headerNames = this._headerNames || {};
@@ -902,7 +946,16 @@ OutgoingMessage.prototype.addTrailers = function(headers) {
       value = headers[key];
     }
 
-    this._trailer += field + ': ' + value + CRLF;
+    if (!lenientHttpHeaders) {
+      if (!_checkIsHttpToken(field)) {
+        throw new TypeError(
+            'Trailer name must be a valid HTTP Token ["' + field + '"]');
+      }
+      if (_checkInvalidHeaderChar(value) === true) {
+        throw new TypeError('The header content contains invalid characters');
+      }
+    }
+    this._trailer += field + ': ' + escapeHeaderValue(value) + CRLF;
   }
 };
 

test/simple/test-http-response-splitting.js

@@ -0,0 +1,53 @@
+var common = require('../common');
+var http = require('http');
+var net = require('net');
+var url = require('url');
+var assert = require('assert');
+
+// Response splitting example, credit: Amit Klein, Safebreach
+var str = '/welcome?lang=bar%c4%8d%c4%8aContent­Length:%200%c4%8d%c4%8a%c' +
+          '4%8d%c4%8aHTTP/1.1%20200%20OK%c4%8d%c4%8aContent­Length:%202' +
+          '0%c4%8d%c4%8aLast­Modified:%20Mon,%2027%20Oct%202003%2014:50:18' +
+          '%20GMT%c4%8d%c4%8aContent­Type:%20text/html%c4%8d%c4%8a%c4%8' +
+          'd%c4%8a%3chtml%3eGotcha!%3c/html%3e';
+
+// Response splitting example, credit: Сковорода Никита Андреевич (@ChALkeR)
+var x = 'fooഊSet-Cookie: foo=barഊഊ<script>alert("Hi!")</script>';
+var y = 'foo⠊Set-Cookie: foo=bar';
+
+var count = 0;
+
+var server = http.createServer(function(req, res) {
+  switch (count++) {
+    case 0:
+      var loc = url.parse(req.url, true).query.lang;
+      assert.throws(common.mustCall(function() {
+        res.writeHead(302, {Location: '/foo?lang=' + loc});
+      }));
+      break;
+    case 1:
+      assert.throws(common.mustCall(function() {
+        res.writeHead(200, {'foo' : x});
+      }));
+      break;
+    case 2:
+      assert.throws(common.mustCall(function() {
+        res.writeHead(200, {'foo' : y});
+      }));
+      break;
+    default:
+      assert.fail(null, null, 'should not get to here.');
+  }
+  if (count === 3)
+    server.close();
+  res.end('ok');
+});
+server.listen(common.PORT, function() {
+  var end = 'HTTP/1.1\r\n\r\n';
+  var client = net.connect({port: common.PORT}, function() {
+    client.write('GET ' + str + ' ' + end);
+    client.write('GET / ' + end);
+    client.write('GET / ' + end);
+    client.end();
+  });
+});