Request Smuggling in WEBrick via bad chunk-size parsing

[kenballus]

When WEBrick receives a request containing an invalid chunk size, it is interpreted as its longest valid prefix. Thus, chunk sizes that begin with 0x are treated as equivalent to 0.

To see why this is a security problem, consider the following payload:

POST / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n0x3a\r\n\r\nGET /evil HTTP/1.1\r\nContent-Length: 23\r\nE: vil\r\nEvil: \r\n\r\n0\r\n\r\nGET / HTTP/1.1\r\n\r\n

WEBrick sees it as a POST request for /, followed by a GET for /evil:

POST / HTTP/1.1\r\n
Transfer-Encoding: chunked\r\n
\r\n
0x3a\r\n
\r\n
GET /evil HTTP/1.1\r\n
Content-Length: 23\r\n
E: vil\r\n
Evil: \r\n
\r\n
0\r\n\r\nGET / HTTP/1.1\r\n\r\n

Unfortunately, some HTTP servers ignore 0x prefixes in chunk sizes due to bad parsing logic. Thus, many servers see a POST request for / and a GET request for /:

POST / HTTP/1.1\r\n
Transfer-Encoding: chunked\r\n
\r\n
0x3a\r\n
\r\nGET /evil HTTP/1.1\r\nContent-Length: 23\r\nE: vil\r\nEvil: \r\n\r\n
0\r\n
\r\n
GET / HTTP/1.1\r\n
\r\n

This discrepancy is exploitable to bypass request filtering rules implemented in reverse proxies that ignore 0x prefixes.

[jeremyevans]

Reviewing RFC 9112 section 7.1 (https://datatracker.ietf.org/doc/html/rfc9112#section-7.1), I think the issue is webrick is interpreting x3a as a chunk extension when it is not a valid chunk extension. Maybe this will work (needs a test added):

diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb
index 7a1686b..23d9b05 100644
--- a/lib/webrick/httprequest.rb
+++ b/lib/webrick/httprequest.rb
@@ -542,7 +542,7 @@ module WEBrick

     def read_chunk_size(socket)
       line = read_line(socket)
-      if /^([0-9a-fA-F]+)(?:;(\S+))?/ =~ line
+      if /\A([0-9a-fA-F]+)(?:;(\S+=\S+))?\r\n\z/ =~ line
         chunk_size = $1.hex
         chunk_ext = $2
         [ chunk_size, chunk_ext ]

[kenballus]

The value of a chunk-ext is optional:

chunk-ext = *( BWS ";" BWS chunk-ext-name [ BWS "=" BWS chunk-ext-val ] )

So I think the patch should maybe be this:

diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb
index 7a1686b..23d9b05 100644
--- a/lib/webrick/httprequest.rb
+++ b/lib/webrick/httprequest.rb
@@ -542,7 +542,7 @@ module WEBrick

     def read_chunk_size(socket)
       line = read_line(socket)
-      if /^([0-9a-fA-F]+)(?:;(\S+))?/ =~ line
+      if /\A([0-9a-fA-F]+)(?:;(\S+(?:=\S+)?))?\r\n\z/ =~ line
         chunk_size = $1.hex
         chunk_ext = $2
         [ chunk_size, chunk_ext ]

[kenballus]

Also, I think the primary issue was with the missing \z from the end of the pattern, since WEBrick wouldn't see the x3a as a chunk-ext without there also being a ;.

[jeremyevans]

Agreed. I'm not sure when I'll have time to work on a test for this, but your patch looks good.

[jeremyevans]

@kenballus I submitted a pull request that uses your fix and adds a test: #125