Ignore illegal response header (#2439)

History.md

@@ -6,7 +6,8 @@
 
 * Bugfixes
   * Your bugfix goes here <Most recent on the top, like GitHub> (#Github Number)
-
+  * Ignore illegal (by Rack spec) response header (#2439)
+
 ## 5.0.3 / 2020-10-26
 
 * Bugfixes

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

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

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,36 +98,29 @@ 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
 
   # testing header value can be separated by \n into line, and each line must not contain characters below 037
   # 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"