Skip to content

Commit

Permalink
Fix puma vulnerability CVE-2024-21647
Browse files Browse the repository at this point in the history
Related CVE: CVE-2024-21647 (https://www.cvedetails.com/cve/CVE-2024-21647/)

This CL mainly cherry picked from the official fix puma@60d5ee3

This change has tested in the local, and followed https://source.corp.google.com/h/looker-internal/codesearch/+/master:cloud-looker/puma/deploying.md as guidence.

Change-Id: Ic390e50c8f6b205b2be072619e462cce425bf637
  • Loading branch information
siyumiG committed Sep 10, 2024
1 parent 80ddb40 commit fde2a97
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
puma (4.3.12.3.looker.custom)
puma (4.3.12.4.looker.custom)
nio4r (~> 2.0)

GEM
Expand Down
27 changes: 27 additions & 0 deletions lib/puma/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ class Client
CHUNK_VALID_ENDING = Const::LINE_END
CHUNK_VALID_ENDING_SIZE = CHUNK_VALID_ENDING.bytesize

# The maximum number of bytes we'll buffer looking for a valid
# chunk header.
MAX_CHUNK_HEADER_SIZE = 4096

# The maximum amount of excess data the client sends
# using chunk size extensions before we abort the connection.
MAX_CHUNK_EXCESS = 16 * 1024

# Content-Length header value validation
CONTENT_LENGTH_VALUE_INVALID = /[^\d]/.freeze

Expand Down Expand Up @@ -441,6 +449,7 @@ def setup_chunked_body(body)
@chunked_body = true
@partial_part_left = 0
@prev_chunk = ""
@excess_cr = 0

@body = Tempfile.new(Const::PUMA_TMP_BASE)
@body.binmode
Expand Down Expand Up @@ -513,6 +522,20 @@ def decode_chunk(chunk)
end
end

# Track the excess as a function of the size of the
# header vs the size of the actual data. Excess can
# go negative (and is expected to) when the body is
# significant.
# The additional of chunk_hex.size and 2 compensates
# for a client sending 1 byte in a chunked body over
# a long period of time, making sure that that client
# isn't accidentally eventually punished.
@excess_cr += (line.size - len - chunk_hex.size - 2)

if @excess_cr >= MAX_CHUNK_EXCESS
raise HttpParserError, "Maximum chunk excess detected"
end

len += 2

part = io.read(len)
Expand Down Expand Up @@ -540,6 +563,10 @@ def decode_chunk(chunk)
@partial_part_left = len - part.size
end
else
if @prev_chunk.size + chunk.size >= MAX_CHUNK_HEADER_SIZE
raise HttpParserError, "maximum size of chunk header exceeded"
end

@prev_chunk = line
return false
end
Expand Down
2 changes: 1 addition & 1 deletion lib/puma/const.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class UnsupportedOption < RuntimeError
# too taxing on performance.
module Const

PUMA_VERSION = VERSION = "4.3.12.3.looker.custom".freeze
PUMA_VERSION = VERSION = "4.3.12.4.looker.custom".freeze
CODE_NAME = "Mysterious Traveller".freeze
PUMA_SERVER_STRING = ['puma', PUMA_VERSION, CODE_NAME].join(' ').freeze

Expand Down
14 changes: 14 additions & 0 deletions test/test_puma_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,20 @@ def test_chunked_request
assert_nil transfer_encoding
end

def test_large_chunked_request_header
server_run app: ->(env) {
[200, {}, [""]]
}

max_chunk_header_size = Puma::Client::MAX_CHUNK_HEADER_SIZE
header = "GET / HTTP/1.1\r\nConnection: close\r\nContent-Length: 200\r\nTransfer-Encoding: chunked\r\n\r\n"
socket = send_http "#{header}1;t#{'x' * (max_chunk_header_size + 2)}"

data = socket.read

assert_match "HTTP/1.1 400 Bad Request\r\nConnection: close\r\nContent-Length: 0\r\n\r\n", data
end

def test_chunked_request_pause_before_value
body = nil
content_length = nil
Expand Down

0 comments on commit fde2a97

Please sign in to comment.