Skip to content

Commit

Permalink
Merge pull request from GHSA-84j7-475p-hp8v
Browse files Browse the repository at this point in the history
header value could inject a CR or LF and inject their own HTTP response.
  • Loading branch information
nateberkopec committed Feb 27, 2020
1 parent bb29fc7 commit 1b17e85
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 0 deletions.
8 changes: 8 additions & 0 deletions benchmarks/wrk/hello.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# You are encouraged to use @ioquatix's wrk fork, located here: https://github.com/ioquatix/wrk

bundle exec bin/puma -t 4 test/rackup/hello.ru &
PID1=$!
sleep 5
wrk -c 4 -d 30 --latency http://localhost:9292

kill $PID1
6 changes: 6 additions & 0 deletions benchmarks/wrk/many_long_headers.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
bundle exec bin/puma -t 4 test/rackup/many_long_headers.ru &
PID1=$!
sleep 5
wrk -c 4 -d 30 --latency http://localhost:9292

kill $PID1
6 changes: 6 additions & 0 deletions benchmarks/wrk/realistic_response.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
bundle exec bin/puma -t 4 test/rackup/realistic_response.ru &
PID1=$!
sleep 5
wrk -c 4 -d 30 --latency http://localhost:9292

kill $PID1
1 change: 1 addition & 0 deletions lib/puma/const.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ module Const
COLON = ": ".freeze

NEWLINE = "\n".freeze
CRLF_REGEX = /[\r\n]/.freeze

HIJACK_P = "rack.hijack?".freeze
HIJACK = "rack.hijack".freeze
Expand Down
2 changes: 2 additions & 0 deletions lib/puma/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,8 @@ def handle_request(req, lines)
status, headers, res_body = @app.call(env)

return :async if req.hijacked
# Checking to see if an attacker is trying to inject headers into the response
headers.reject! { |_k, v| CRLF_REGEX =~ v.to_s }

status = status.to_i

Expand Down
9 changes: 9 additions & 0 deletions test/rackup/many_long_headers.ru
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
require 'securerandom'

long_header_hash = {}

30.times do |i|
long_header_hash["X-My-Header-#{i}"] = SecureRandom.hex(1000)
end

run lambda { |env| [200, long_header_hash, ["Hello World"]] }
11 changes: 11 additions & 0 deletions test/rackup/realistic_response.ru
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
require 'securerandom'

long_header_hash = {}

25.times do |i|
long_header_hash["X-My-Header-#{i}"] = SecureRandom.hex(25)
end

response = SecureRandom.hex(100_000) # A 100kb document

run lambda { |env| [200, long_header_hash.dup, [response.dup]] }
19 changes: 19 additions & 0 deletions test/test_puma_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -740,4 +740,23 @@ def test_empty_header_values

assert_equal "HTTP/1.0 200 OK\r\nX-Empty-Header: \r\n\r\n", data
end

# https://github.com/ruby/ruby/commit/d9d4a28f1cdd05a0e8dabb36d747d40bbcc30f16
def test_prevent_response_splitting_headers
server_run app: ->(_) { [200, {'X-header' => "malicious\r\nCookie: hack"}, ["Hello"]] }
data = send_http_and_read "HEAD / HTTP/1.0\r\n\r\n"
refute_match 'hack', data
end

def test_prevent_response_splitting_headers_cr
server_run app: ->(_) { [200, {'X-header' => "malicious\rCookie: hack"}, ["Hello"]] }
data = send_http_and_read "HEAD / HTTP/1.0\r\n\r\n"
refute_match 'hack', data
end

def test_prevent_response_splitting_headers_lf
server_run app: ->(_) { [200, {'X-header' => "malicious\nCookie: hack"}, ["Hello"]] }
data = send_http_and_read "HEAD / HTTP/1.0\r\n\r\n"
refute_match 'hack', data
end
end

0 comments on commit 1b17e85

Please sign in to comment.