From: Evan Phoenix Date: Thu, 19 Sep 2024 04:56:07 +0000 (-0700) Subject: [PATCH] Merge commit from fork X-Git-Tag: archive/raspbian/5.6.5-3+rpi1+deb12u1^2~1 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=c043b3e2e257b2b6cf8155c8986ae33ea532ba26;p=puma.git [PATCH] Merge commit from fork * Prevent underscores from clobbering hyphen headers * Special case encoding headers to prevent app confusion * Handle _ as , in jruby as well * Silence RuboCop offense --------- Co-authored-by: Patrik Ragnarsson Gbp-Pq: Name CVE-2024-45614.patch --- diff --git a/ext/puma_http11/org/jruby/puma/Http11.java b/ext/puma_http11/org/jruby/puma/Http11.java index cd7a5d3..0c4f79e 100644 --- a/ext/puma_http11/org/jruby/puma/Http11.java +++ b/ext/puma_http11/org/jruby/puma/Http11.java @@ -99,6 +99,8 @@ public class Http11 extends RubyObject { int bite = b.get(i) & 0xFF; if(bite == '-') { b.set(i, (byte)'_'); + } else if(bite == '_') { + b.set(i, (byte)','); } else { b.set(i, (byte)Character.toUpperCase(bite)); } diff --git a/lib/puma/const.rb b/lib/puma/const.rb index cc0fe95..12b8bfd 100644 --- a/lib/puma/const.rb +++ b/lib/puma/const.rb @@ -244,6 +244,14 @@ module Puma # header values can contain HTAB? ILLEGAL_HEADER_VALUE_REGEX = /[\x00-\x08\x0A-\x1F]/.freeze + # The keys of headers that should not be convert to underscore + # normalized versions. These headers are ignored at the request reading layer, + # but if we normalize them after reading, it's just confusing for the application. + UNMASKABLE_HEADERS = { + "HTTP_TRANSFER,ENCODING" => true, + "HTTP_CONTENT,LENGTH" => true, + } + # Banned keys of response header BANNED_HEADER_KEY = /\A(rack\.|status\z)/.freeze diff --git a/lib/puma/request.rb b/lib/puma/request.rb index 8c7b008..bedabe3 100644 --- a/lib/puma/request.rb +++ b/lib/puma/request.rb @@ -318,6 +318,11 @@ module Puma # compatibility, we'll convert them back. This code is written to # avoid allocation in the common case (ie there are no headers # with `,` in their names), that's why it has the extra conditionals. + # + # @note If a normalized version of a `,` header already exists, we ignore + # the `,` version. This prevents clobbering headers managed by proxies + # but not by clients (Like X-Forwarded-For). + # # @param env [Hash] see Puma::Client#env, from request, modifies in place # @version 5.0.3 # @@ -326,23 +331,30 @@ module Puma to_add = nil env.each do |k,v| - if k.start_with?("HTTP_") and k.include?(",") and k != "HTTP_TRANSFER,ENCODING" + if k.start_with?("HTTP_") and k.include?(",") and !UNMASKABLE_HEADERS.key?(k) if to_delete to_delete << k else to_delete = [k] end + new_k = k.tr(",", "_") + if env.key?(new_k) + next + end + unless to_add to_add = {} end - to_add[k.tr(",", "_")] = v + to_add[new_k] = v end end if to_delete to_delete.each { |k| env.delete(k) } + end + if to_add env.merge! to_add end end diff --git a/test/test_normalize.rb b/test/test_normalize.rb new file mode 100644 index 0000000..60e61c3 --- /dev/null +++ b/test/test_normalize.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require_relative "helper" + +require "puma/request" + +class TestNormalize < Minitest::Test + parallelize_me! + + include Puma::Request + + def test_comma_headers + env = { + "HTTP_X_FORWARDED_FOR" => "1.1.1.1", + "HTTP_X_FORWARDED,FOR" => "2.2.2.2", + } + + req_env_post_parse env + + expected = { + "HTTP_X_FORWARDED_FOR" => "1.1.1.1", + } + + assert_equal expected, env + + # Test that the iteration order doesn't matter + + env = { + "HTTP_X_FORWARDED,FOR" => "2.2.2.2", + "HTTP_X_FORWARDED_FOR" => "1.1.1.1", + } + + req_env_post_parse env + + expected = { + "HTTP_X_FORWARDED_FOR" => "1.1.1.1", + } + + assert_equal expected, env + end + + def test_unmaskable_headers + env = { + "HTTP_CONTENT,LENGTH" => "100000", + "HTTP_TRANSFER,ENCODING" => "chunky" + } + + req_env_post_parse env + + expected = { + "HTTP_CONTENT,LENGTH" => "100000", + "HTTP_TRANSFER,ENCODING" => "chunky" + } + + assert_equal expected, env + end +end diff --git a/test/test_request_invalid.rb b/test/test_request_invalid.rb index 8e9295b..c6aa91a 100644 --- a/test/test_request_invalid.rb +++ b/test/test_request_invalid.rb @@ -216,4 +216,32 @@ class TestRequestInvalid < Minitest::Test assert_status data end + + def test_underscore_header_1 + hdrs = [ + "X-FORWARDED-FOR: 1.1.1.1", # proper + "X-FORWARDED-FOR: 2.2.2.2", # proper + "X_FORWARDED-FOR: 3.3.3.3", # invalid, contains underscore + "Content-Length: 5", + ].join "\r\n" + + response = send_http_and_read "#{GET_PREFIX}#{hdrs}\r\n\r\nHello\r\n\r\n" + + assert_includes response, "HTTP_X_FORWARDED_FOR = 1.1.1.1, 2.2.2.2" + refute_includes response, "3.3.3.3" + end + + def test_underscore_header_2 + hdrs = [ + "X_FORWARDED-FOR: 3.3.3.3", # invalid, contains underscore + "X-FORWARDED-FOR: 2.2.2.2", # proper + "X-FORWARDED-FOR: 1.1.1.1", # proper + "Content-Length: 5", + ].join "\r\n" + + response = send_http_and_read "#{GET_PREFIX}#{hdrs}\r\n\r\nHello\r\n\r\n" + + assert_includes response, "HTTP_X_FORWARDED_FOR = 2.2.2.2, 1.1.1.1" + refute_includes response, "3.3.3.3" + end end