http, querystring: added limits to prevent DoS

nodejs · Jan 15, 2012 · 8a98c2f · bnoordhuis · Jan 16, 2012 · windyrobin
1 parent 93465d3
commit 8a98c2f
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 6 deletions.
diff --git a/doc/api/http.markdown b/doc/api/http.markdown
@@ -143,6 +143,12 @@ Stops the server from accepting new connections.
 See [net.Server.close()](net.html#server.close).
 
 
+### server.maxHeadersCount
+
+Limits maximum incoming headers count, equal to 1000 by default. If set to 0 -
+no limit will be applied.
+
+
 ## http.ServerRequest
 
 This object is created internally by a HTTP server -- not by

diff --git a/doc/api/querystring.markdown b/doc/api/querystring.markdown
@@ -19,12 +19,15 @@ Example:
     // returns
     'foo:bar;baz:qux'
 
-### querystring.parse(str, [sep], [eq])
+### querystring.parse(str, [sep], [eq], [options])
 
 Deserialize a query string to an object.
 Optionally override the default separator (`'&'`) and assignment (`'='`)
 characters.
 
+Options object may contain `maxKeys` property (equal to 1000 by default), it'll
+be used to limit processed keys. Set it to 0 to remove key count limitation.
+
 Example:
 
     querystring.parse('foo=bar&baz=qux&baz=quux&corge')

diff --git a/lib/http.js b/lib/http.js
@@ -43,6 +43,10 @@ var parsers = new FreeList('parsers', 1000, function() {
   parser._headers = [];
   parser._url = '';
 
+  // Limit incoming headers count as it may cause
+  // hash collision DoS
+  parser.maxHeadersCount = 1000;
+
   // Only called in the slow case where slow means
   // that the request headers were either fragmented
   // across multiple TCP packets or too large to be
@@ -78,7 +82,14 @@ var parsers = new FreeList('parsers', 1000, function() {
     parser.incoming.httpVersion = info.versionMajor + '.' + info.versionMinor;
     parser.incoming.url = url;
 
-    for (var i = 0, n = headers.length; i < n; i += 2) {
+    var n = headers.length;
+
+    // If parser.maxHeadersCount <= 0 - assume that there're no limit
+    if (parser.maxHeadersCount > 0) {
+      n = Math.min(n, parser.maxHeadersCount << 1);
+    }
+
+    for (var i = 0; i < n; i += 2) {
       var k = headers[i];
       var v = headers[i + 1];
       parser.incoming._addHeaderLine(k.toLowerCase(), v);
@@ -1158,6 +1169,11 @@ ClientRequest.prototype.onSocket = function(socket) {
     parser.incoming = null;
     req.parser = parser;
 
+    // Propagate headers limit from request object to parser
+    if (req.maxHeadersCount) {
+      parser.maxHeadersCount = req.maxHeadersCount;
+    }
+
     socket._httpMessage = req;
     // Setup "drain" propogation.
     httpSocketSetup(socket);
@@ -1444,6 +1460,11 @@ function connectionListener(socket) {
   parser.socket = socket;
   parser.incoming = null;
 
+  // Propagate headers limit from server instance to parser
+  if (this.maxHeadersCount) {
+    parser.maxHeadersCount = this.maxHeadersCount;
+  }
+
   socket.addListener('error', function(e) {
     self.emit('clientError', e);
   });

diff --git a/lib/querystring.js b/lib/querystring.js
@@ -160,16 +160,24 @@ QueryString.stringify = QueryString.encode = function(obj, sep, eq, name) {
 };
 
 // Parse a key=val string.
-QueryString.parse = QueryString.decode = function(qs, sep, eq) {
+QueryString.parse = QueryString.decode = function(qs, sep, eq, options) {
   sep = sep || '&';
   eq = eq || '=';
-  var obj = {};
+  var obj = {},
+      maxKeys = options && options.maxKeys || 1000;
 
   if (typeof qs !== 'string' || qs.length === 0) {
     return obj;
   }
 
-  qs.split(sep).forEach(function(kvp) {
+  qs = qs.split(sep);
+
+  // maxKeys <= 0 means that we should not limit keys count
+  if (maxKeys > 0) {
+    qs = qs.slice(0, maxKeys);
+  }
+
+  qs.forEach(function(kvp) {
     var x = kvp.split(eq);
     var k = QueryString.unescape(x[0], true);
     var v = QueryString.unescape(x.slice(1).join(eq), true);

diff --git a/test/simple/test-querystring.js b/test/simple/test-querystring.js
@@ -183,6 +183,12 @@ assert.equal(f, 'a:b;q:x%3Ay%3By%3Az');
 assert.deepEqual({}, qs.parse());
 
 
+// Test limiting
+assert.equal(
+  Object.keys(qs.parse('a=1&b=1&c=1', null, null, { maxKeys: 1 })).length,
+  1
+);
+
 
 var b = qs.unescapeBuffer('%d3%f2Ug%1f6v%24%5e%98%cb' +
                           '%0d%ac%a2%2f%9d%eb%d8%a2%e6');
@@ -207,4 +213,3 @@ assert.equal(0xeb, b[16]);
 assert.equal(0xd8, b[17]);
 assert.equal(0xa2, b[18]);
 assert.equal(0xe6, b[19]);
-