Request Smuggling in WEBrick via bad chunk-size parsing about webrick HOT 5 CLOSED

kenballus commented on July 30, 2024

Request Smuggling in WEBrick via bad chunk-size parsing

from webrick.

Comments (5)

jeremyevans commented on July 30, 2024 1

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

from webrick.

jeremyevans commented on July 30, 2024

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 ]

from webrick.

kenballus commented on July 30, 2024

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 ]

from webrick.

kenballus commented on July 30, 2024

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 ;.

from webrick.

jeremyevans commented on July 30, 2024

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

from webrick.

Recommend Projects