diff --git a/History.md b/History.md index b4b2988e7b..7d0eb2e216 100644 --- a/History.md +++ b/History.md @@ -6,7 +6,8 @@ * Bugfixes * Your bugfix goes here (#Github Number) - + * Ignore illegal (by Rack spec) response header (#2439) + ## 5.0.3 / 2020-10-26 * Bugfixes diff --git a/lib/puma/const.rb b/lib/puma/const.rb index a498f10b68..a67e294b25 100644 --- a/lib/puma/const.rb +++ b/lib/puma/const.rb @@ -228,7 +228,6 @@ module Const COLON = ": ".freeze NEWLINE = "\n".freeze - HTTP_INJECTION_REGEX = /[\r\n]/.freeze HIJACK_P = "rack.hijack?".freeze HIJACK = "rack.hijack".freeze @@ -239,5 +238,13 @@ module Const # Mininum interval to checks worker health WORKER_CHECK_INTERVAL = 5 + # Illegal character in the key or value of response header + DQUOTE = "\"".freeze + HTTP_HEADER_DELIMITER = Regexp.escape("(),/:;<=>?@[]{}").freeze + ILLEGAL_HEADER_KEY_REGEX = /[\u0000-\u0025|#{DQUOTE}|#{HTTP_HEADER_DELIMITER}]/.freeze + ILLEGAL_HEADER_VALUE_REGEX = /[\000-\037]/.freeze + + # Banned keys of response header + BANNED_HEADER_KEY = /rack.|status/.freeze end end diff --git a/lib/puma/request.rb b/lib/puma/request.rb index 296ebb116c..415d15cc75 100644 --- a/lib/puma/request.rb +++ b/lib/puma/request.rb @@ -282,13 +282,20 @@ def normalize_env(env, client) end # private :normalize_env + # @param header_key [#to_s] + # @return [Boolean] + # + def illegal_header_key?(header_key) + !!(ILLEGAL_HEADER_KEY_REGEX =~ header_key.to_s) + end + # @param header_value [#to_s] # @return [Boolean] # - def possible_header_injection?(header_value) - !!(HTTP_INJECTION_REGEX =~ header_value.to_s) + def illegal_header_value?(header_value) + !!(ILLEGAL_HEADER_VALUE_REGEX =~ header_value.to_s) end - private :possible_header_injection? + private :illegal_header_key?, :illegal_header_value? # Fixup any headers with `,` in the name to have `_` now. We emit # headers with `,` in them during the parse phase to avoid ambiguity @@ -334,9 +341,11 @@ def req_env_post_parse(env) def str_early_hints(headers) eh_str = "HTTP/1.1 103 Early Hints\r\n".dup headers.each_pair do |k, vs| + next if illegal_header_key?(k) + if vs.respond_to?(:to_s) && !vs.to_s.empty? vs.to_s.split(NEWLINE).each do |v| - next if possible_header_injection?(v) + next if illegal_header_value?(v) eh_str << "#{k}: #{v}\r\n" end else @@ -399,9 +408,11 @@ def str_headers(env, status, headers, res_info, lines) res_info[:response_hijack] = nil headers.each do |k, vs| + next if illegal_header_key?(k) + case k.downcase when CONTENT_LENGTH2 - next if possible_header_injection?(vs) + next if illegal_header_value?(vs) res_info[:content_length] = vs next when TRANSFER_ENCODING @@ -410,11 +421,13 @@ def str_headers(env, status, headers, res_info, lines) when HIJACK res_info[:response_hijack] = vs next + when BANNED_HEADER_KEY + next end if vs.respond_to?(:to_s) && !vs.to_s.empty? vs.to_s.split(NEWLINE).each do |v| - next if possible_header_injection?(v) + next if illegal_header_value?(v) lines.append k, colon, v, line_ending end else diff --git a/test/test_response_header.rb b/test/test_response_header.rb index ae1cc0d3e5..0f44d1a623 100644 --- a/test/test_response_header.rb +++ b/test/test_response_header.rb @@ -84,15 +84,11 @@ def assert_ignore_header(name, value, opts={}) # The header must not contain a Status key. def test_status_key - skip 'implement later' - assert_ignore_header("Status", "500") end # Special headers starting “rack.” are for communicating with the server, and must not be sent back to the client. def test_rack_key - skip 'implement later' - assert_ignore_header("rack.command_to_server_only", "work") end @@ -102,12 +98,10 @@ def test_rack_key # Header keys will be set through two ways: Regular and early hints. def test_illegal_character_in_key - skip 'implement later' assert_ignore_header("\"F\u0000o\u0025(@o}", "Boo") end def test_illegal_character_in_key_when_early_hints - skip 'implement later' assert_ignore_header("\"F\u0000o\u0025(@o}", "Boo", early_hints: true) end @@ -115,23 +109,18 @@ def test_illegal_character_in_key_when_early_hints # Header values can be set through three ways: Regular, early hints and a special case for overriding content-length def test_illegal_character_in_value - skip 'implement later' assert_ignore_header("X-header", "First \000Lin\037e") end def test_illegal_character_in_value_when_early_hints - skip 'implement later' assert_ignore_header("X-header", "First \000Lin\037e", early_hints: true) end def test_illegal_character_in_value_when_override_content_length - skip 'implement later' assert_ignore_header("Content-Length", "\037") end def test_illegal_character_in_value_when_newline - skip 'implement later' - server_run app: ->(env) { [200, {'X-header' => "First\000 line\nSecond Lin\037e"}, ["Hello"]] } data = send_http_and_read "GET / HTTP/1.0\r\n\r\n"