about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2010-04-19 13:40:37 -0700
committerEric Wong <normalperson@yhbt.net>2010-04-19 13:49:02 -0700
commitde58973a6025f799ee7897a2cf58bb01eca87631 (patch)
tree9345e57df498f43956310f531e75e7f4cf560bbd
parent5dd4c12411b5567fb340e897d0b544dfff7fa7a2 (diff)
downloadunicorn-de58973a6025f799ee7897a2cf58bb01eca87631.tar.gz
...instead of tripping an assertion.

This fixes a potential denial-of-service for servers exposed directly
to untrusted clients.

This bug does not affect supported Unicorn deployments as Unicorn is
only supported with trusted clients (such as nginx) on a LAN.  nginx is
known to reject clients that send invalid Content-Length headers, so any
deployments on a trusted LAN and/or behind nginx are safe.

Servers affected by this bug include (but are not limited to) Rainbows!
and Zbatery.  This does not affect Thin nor Mongrel which never got
request body filtering treatment that the Unicorn HTTP parser got in
August 2009.
-rw-r--r--ext/unicorn_http/c_util.h8
-rw-r--r--test/unit/test_http_parser_ng.rb20
2 files changed, 26 insertions, 2 deletions
diff --git a/ext/unicorn_http/c_util.h b/ext/unicorn_http/c_util.h
index 8542b3d..ab1fc0e 100644
--- a/ext/unicorn_http/c_util.h
+++ b/ext/unicorn_http/c_util.h
@@ -108,8 +108,12 @@ static off_t parse_length(const char *value, size_t length)
 {
   off_t rv;
 
-  for (rv = 0; length-- && rv >= 0; ++value)
-    rv = step_incr(rv, *value, 10);
+  for (rv = 0; length-- && rv >= 0; ++value) {
+    if (*value >= '0' && *value <= '9')
+      rv = step_incr(rv, *value, 10);
+    else
+      return -1;
+  }
 
   return rv;
 }
diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb
index 4980249..3b9111f 100644
--- a/test/unit/test_http_parser_ng.rb
+++ b/test/unit/test_http_parser_ng.rb
@@ -416,4 +416,24 @@ class HttpParserNgTest < Test::Unit::TestCase
     end
   end
 
+  def test_negative_content_length
+    req = {}
+    str = "PUT / HTTP/1.1\r\n" \
+          "Content-Length: -1\r\n" \
+          "\r\n"
+    assert_raises(HttpParserError) do
+      @parser.headers(req, str)
+    end
+  end
+
+  def test_invalid_content_length
+    req = {}
+    str = "PUT / HTTP/1.1\r\n" \
+          "Content-Length: zzzzz\r\n" \
+          "\r\n"
+    assert_raises(HttpParserError) do
+      @parser.headers(req, str)
+    end
+  end
+
 end