From b5dbf4aa582ec02af29d2a58c8af1c6d722b84ba Mon Sep 17 00:00:00 2001 From: Arne Hartherz Date: Thu, 6 Mar 2025 15:10:57 +0100 Subject: [PATCH 1/6] Backport Rack fix for CVE-2025-27111 --- lib/rack/sendfile.rb | 2 +- test/spec_sendfile.rb | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/rack/sendfile.rb b/lib/rack/sendfile.rb index 4a9b428..fbecbc2 100644 --- a/lib/rack/sendfile.rb +++ b/lib/rack/sendfile.rb @@ -135,7 +135,7 @@ def call(env) end when '', nil else - env['rack.errors'].puts "Unknown x-sendfile variation: '#{type}'.\n" + env['rack.errors'].puts "Unknown x-sendfile variation: #{type.inspect}" end end [status, headers, body] diff --git a/test/spec_sendfile.rb b/test/spec_sendfile.rb index 7c9acd6..affe430 100644 --- a/test/spec_sendfile.rb +++ b/test/spec_sendfile.rb @@ -46,6 +46,30 @@ def open_file(path) end end + it "does nothing and logs to rack.errors when incorrect X-Sendfile-Type header present" do + io = StringIO.new + request 'HTTP_X_SENDFILE_TYPE' => 'X-Banana', 'rack.errors' => io do |response| + response.should.be.ok + response.body.should.equal 'Hello World' + response.headers.should.not.include 'X-Sendfile' + + io.rewind + io.read.should.equal "Unknown x-sendfile variation: \"X-Banana\"\n" + end + end + + it "does not send multi-line headers for invalid multi-line X-Sendfile-Type values" do + io = StringIO.new + request 'HTTP_X_SENDFILE_TYPE' => "Hello\nCVE-2025-27111", 'rack.errors' => io do |response| + response.should.be.ok + response.body.should.equal 'Hello World' + response.headers.should.not.include 'X-Sendfile' + + io.rewind + io.read.should.equal "Unknown x-sendfile variation: \"Hello\\nCVE-2025-27111\"\n" + end + end + it "sets X-Sendfile response header and discards body" do request 'HTTP_X_SENDFILE_TYPE' => 'X-Sendfile' do |response| response.should.be.ok From a2f053b19fe7c9e3f3799632aef4a673704b8388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20H=C3=A4usele?= Date: Fri, 9 May 2025 13:20:44 +0200 Subject: [PATCH 2/6] Backport CVE-2025-46727 --- lib/rack/utils.rb | 38 ++++++++++++++++++++++++++++++++++++-- test/spec_utils.rb | 22 ++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index f84cd1c..86a3f7d 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -31,6 +31,10 @@ class ParameterTypeError < TypeError; end # sequence. class InvalidParameterError < ArgumentError; end + # QueryLimitError is for errors raised when the query provided exceeds one + # of the query parser limits. + class QueryLimitError < RangeError; end + # URI escapes. (CGI style space to +) def escape(s) URI.encode_www_form_component(s) @@ -64,6 +68,8 @@ class << self attr_accessor :param_depth_limit attr_accessor :multipart_total_part_limit attr_accessor :multipart_file_limit + attr_accessor :bytesize_limit # CVE-2025-46727 + attr_accessor :params_limit # CVE-2025-46727 # multipart_part_limit is the original name of multipart_file_limit, but # the limit only counts parts with filenames. @@ -89,6 +95,34 @@ class << self # many can lead to excessive memory use and parsing time. self.multipart_total_part_limit = (ENV['RACK_MULTIPART_TOTAL_PART_LIMIT'] || 4096).to_i + # This sets the default for the maximum query string bytesize that we will attempt to parse. + # Attempts to use a query string that exceeds this number of bytes will result in a + # `Rack::Utils::QueryLimitError` exception. + self.bytesize_limit = (ENV['RACK_QUERY_PARSER_BYTESIZE_LIMIT'] || 4194304).to_i + + # This variable sets the default for the maximum number of query + # parameters that we will attempt to parse. Attempts to use a + # query string with more than this many query parameters will result in a + # `Rack::Utils::QueryLimitError` exception. + self.params_limit = (ENV['RACK_QUERY_PARSER_PARAMS_LIMIT'] || 4096).to_i + + def check_query_string(qs, sep) + if qs + if qs.bytesize > Rack::Utils.bytesize_limit + raise QueryLimitError, "total query size (#{qs.bytesize}) exceeds limit (#{Rack::Utils.bytesize_limit})" + end + + if (param_count = qs.count(sep.is_a?(String) ? sep : '&')) >= Rack::Utils.params_limit + raise QueryLimitError, "total number of query parameters (#{param_count+1}) exceeds limit (#{Rack::Utils.params_limit})" + end + + qs + else + '' + end + end + module_function :check_query_string + # Stolen from Mongrel, with some small modifications: # Parses a query string by breaking it up at the '&' # and ';' characters. You can also use this to parse @@ -99,7 +133,7 @@ def parse_query(qs, d = nil, &unescaper) params = KeySpaceConstrainedParams.new - (qs || '').split(d ? /[#{d}] */n : DEFAULT_SEP).each do |p| + check_query_string(qs, d).split(d ? /[#{d}] */n : DEFAULT_SEP).each do |p| next if p.empty? k, v = p.split('=', 2).map(&unescaper) @@ -126,7 +160,7 @@ def parse_query(qs, d = nil, &unescaper) def parse_nested_query(qs, d = nil) params = KeySpaceConstrainedParams.new - (qs || '').split(d ? /[#{d}] */n : DEFAULT_SEP).each do |p| + check_query_string(qs, d).split(d ? /[#{d}] */n : DEFAULT_SEP).each do |p| k, v = p.split('=', 2).map { |s| unescape(s) } normalize_params(params, k, v) diff --git a/test/spec_utils.rb b/test/spec_utils.rb index 7339d6f..46831c1 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -462,6 +462,28 @@ def kcodeu should "clean slash only paths" do Rack::Utils.clean_path_info("/").should.equal "/" end + + should "respect bytesize_limit to specify maximum size of query string to parse" do + Rack::Utils.bytesize_limit = 3 + Rack::Utils.parse_query("a=a").should.equal({"a" => "a"}) + Rack::Utils.parse_nested_query("a=a").should.equal({"a" => "a"}) + Rack::Utils.parse_nested_query("a=a", '&').should.equal({"a" => "a"}) + proc { Rack::Utils.parse_query("a=aa") }.should.raise(Rack::Utils::QueryLimitError) + proc { Rack::Utils.parse_nested_query("a=aa") }.should.raise(Rack::Utils::QueryLimitError) + proc { Rack::Utils.parse_nested_query("a=aa", '&') }.should.raise(Rack::Utils::QueryLimitError) + Rack::Utils.bytesize_limit = 4194304 + end + + it "accepts params_limit to specify maximum number of query parameters to parse" do + Rack::Utils.params_limit = 2 + Rack::Utils.parse_query("a=a&b=b").should.equal({"a" => "a", "b" => "b"}) + Rack::Utils.parse_nested_query("a=a&b=b").should.equal({"a" => "a", "b" => "b"}) + Rack::Utils.parse_nested_query("a=a&b=b", '&').should.equal({"a" => "a", "b" => "b"}) + proc { Rack::Utils.parse_query("a=a&b=b&c=c") }.should.raise(Rack::Utils::QueryLimitError) + proc { Rack::Utils.parse_nested_query("a=a&b=b&c=c", '&') }.should.raise(Rack::Utils::QueryLimitError) + proc { Rack::Utils.parse_query("b[]=a&b[]=b&b[]=c") }.should.raise(Rack::Utils::QueryLimitError) + Rack::Utils.params_limit = 4096 + end end describe Rack::Utils, "byte_range" do From 3dd65a58e72139aac1e54c57d518e3a114732df5 Mon Sep 17 00:00:00 2001 From: Tobias Kraze Date: Thu, 9 Oct 2025 13:09:09 +0200 Subject: [PATCH 3/6] Rack: backport fix for CVE-2025-61772; test for CVE-2025-61770 --- lib/rack/mock.rb | 2 +- lib/rack/multipart/parser.rb | 8 +++- test/spec_multipart.rb | 81 ++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/lib/rack/mock.rb b/lib/rack/mock.rb index 1a1add2..383b6fa 100644 --- a/lib/rack/mock.rb +++ b/lib/rack/mock.rb @@ -134,7 +134,7 @@ def self.env_for(uri="", opts={}) rack_input.set_encoding(Encoding::BINARY) if rack_input.respond_to?(:set_encoding) env['rack.input'] = rack_input - env["CONTENT_LENGTH"] ||= env["rack.input"].length.to_s + env["CONTENT_LENGTH"] ||= env["rack.input"].length.to_s if env["rack.input"].respond_to?(:length) opts.each { |field, value| env[field] = value if String === field diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index 153530d..f21a4b8 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -7,6 +7,7 @@ class MultipartTotalPartLimitError < StandardError; end class Parser BUFSIZE = 16384 + MIME_HEADER_BYTESIZE_LIMIT = 64 * 1024 DUMMY = Struct.new(:parse).new def self.create(env) @@ -70,7 +71,7 @@ def parse parts += 1 if parts >= Utils.multipart_total_part_limit close_tempfiles - raise MultipartTotalPartLimitError, 'Maximum total multiparts in content reached' + raise MultipartTotalPartLimitError, 'Maximum total multiparts in content reached' end end @@ -113,7 +114,7 @@ def fast_forward_to_first_boundary return if read_buffer == full_boundary end - raise EOFError, "bad content body" if Utils.bytesize(@buf) >= @bufsize + raise EOFError, "multipart boundary not found within limit" if Utils.bytesize(@buf) >= @bufsize end end @@ -159,6 +160,9 @@ def get_current_head_and_filename_and_content_type_and_name_and_body raise EOFError, "bad content body" if content.nil? || content.empty? @buf << content + + raise EOFError, "multipart mime part header too large" if @buf.size > MIME_HEADER_BYTESIZE_LIMIT + @content_length -= content.size if @content_length end diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index a56eee9..de39f95 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -139,6 +139,87 @@ def rd.length wr.close end + should "reject excessive data before boundary" do + rd, wr = IO.pipe + def rd.rewind; end + wr.sync = true + + thr = Thread.new do + begin + longer = "0123456789" * 1024 * 1024 + (1024 * 1024).times do + wr.write(longer) + end + + wr.write("\r\n\r\n--AaB03x") + wr.write("\r\n") + wr.write('content-disposition: form-data; name="a"; filename="a.txt"') + wr.write("\r\n") + wr.write("content-type: text/plain\r\n") + wr.write("\r\na") + wr.write("--AaB03x--\r\n") + wr.close + rescue => err # this is EPIPE if Rack shuts us down + err + end + end + + fixture = { + "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x", + :input => rd, + } + + env = Rack::MockRequest.env_for '/', fixture + lambda { + Rack::Multipart.parse_multipart(env) + }.should.raise(EOFError).message.should.equal "multipart boundary not found within limit" + rd.close + + err = thr.value + err.should.be.instance_of Errno::EPIPE + wr.close + end + + should "reject excessive mime header size" do + rd, wr = IO.pipe + def rd.rewind; end + wr.sync = true + + thr = Thread.new do + begin + wr.write("\r\n\r\n--AaB03x") + wr.write("\r\n") + wr.write('content-disposition: form-data; name="a"; filename="a.txt"') + wr.write("\r\n") + wr.write("content-type: text/plain\r\n") + longer = "0123456789" + (1024 * 1024).times do + wr.write(longer) + end + wr.write("\r\n\r\na") + wr.write("--AaB03x--\r\n") + wr.close + rescue => err # this is EPIPE if Rack shuts us down + err + end + end + + fixture = { + "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x", + :input => rd, + } + + env = Rack::MockRequest.env_for '/', fixture + lambda { + Rack::Multipart.parse_multipart(env) + }.should.raise(EOFError).message.should.equal "multipart mime part header too large" + rd.close + + err = thr.value + err.should.be.instance_of Errno::EPIPE + wr.close + end + should "parse multipart upload with text file" do env = Rack::MockRequest.env_for("/", multipart_fixture(:text)) params = Rack::Multipart.parse_multipart(env) From 969611c0600b048e48288badabe82258cf8d93e5 Mon Sep 17 00:00:00 2001 From: Tobias Kraze Date: Thu, 9 Oct 2025 13:48:41 +0200 Subject: [PATCH 4/6] Rack: backport fix for CVE-2025-61771 --- README.md | 9 +++ lib/rack/multipart/parser.rb | 17 +++++- lib/rack/utils.rb | 6 ++ test/spec_multipart.rb | 104 +++++++++++++++++++++++++++++++++++ 4 files changed, 134 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4b8c7fc..e69d1bd 100644 --- a/README.md +++ b/README.md @@ -245,6 +245,15 @@ Set to 0 for no limit. Can also be set via the `RACK_MULTIPART_TOTAL_PART_LIMIT` environment variable. +### multipart_buffered_upload_bytesize_limit + +The limit of the bytesize of all multipart parts (header and body), excluding the (body) of parts with a "filename". + +Defaults to 16 MB, which means it is not possible for multipart forms to contain form data of a total size greater than 16 MB. Uploaded files can be larger. + +Can also be set via the `RACK_MULTIPART_BUFFERED_UPLOAD_BYTESIZE_LIMIT` environment variable. + + ## History See . diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index f21a4b8..825015d 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -48,6 +48,8 @@ def initialize(boundary, io, content_length, env, tempfile, bufsize) @rx = /(?:#{EOL})?#{Regexp.quote(@boundary)}(#{EOL}|--)/n @full_boundary = @boundary + EOL + + @retained_size = 0 end def parse @@ -78,6 +80,7 @@ def parse # Save the rest. if i = @buf.index(rx) body << @buf.slice!(0, i) + update_retained_size(i) unless filename @buf.slice!(0, @boundary_size+2) @content_length = -1 if $1 == "--" @@ -134,6 +137,7 @@ def get_current_head_and_filename_and_content_type_and_name_and_body @buf.slice!(0, 2) # Second \r\n + update_retained_size(head.bytesize) content_type = head[MULTIPART_CONTENT_TYPE, 1] name = head[MULTIPART_CONTENT_DISPOSITION, 1] || head[MULTIPART_CONTENT_ID, 1] @@ -152,8 +156,10 @@ def get_current_head_and_filename_and_content_type_and_name_and_body end # Save the read body part. - if head && (@boundary_size+4 < @buf.size) - body << @buf.slice!(0, @buf.size - (@boundary_size+4)) + size_to_read = @buf.size - (@boundary_size+4) + if head && size_to_read > 0 + body << @buf.slice!(0, size_to_read) + update_retained_size(size_to_read) unless filename end content = @io.read(@content_length && @bufsize >= @content_length ? @content_length : @bufsize) @@ -269,6 +275,13 @@ def get_data(filename, body, content_type, name, head) yield data end + + def update_retained_size(size) + @retained_size += size + if @retained_size > Utils.buffered_upload_bytesize_limit + raise EOFError, "multipart data over retained size limit" + end + end end end end diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 86a3f7d..8017505 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -70,6 +70,7 @@ class << self attr_accessor :multipart_file_limit attr_accessor :bytesize_limit # CVE-2025-46727 attr_accessor :params_limit # CVE-2025-46727 + attr_accessor :buffered_upload_bytesize_limit # CVE-2025-61771 # multipart_part_limit is the original name of multipart_file_limit, but # the limit only counts parts with filenames. @@ -106,6 +107,11 @@ class << self # `Rack::Utils::QueryLimitError` exception. self.params_limit = (ENV['RACK_QUERY_PARSER_PARAMS_LIMIT'] || 4096).to_i + # This variable sets the maximum total size of all parts and headers + # of a multipart request. Parts with filenames are written to tempfiles + # and do not count. Defaults to 16 MB. + self.buffered_upload_bytesize_limit = (ENV['RACK_MULTIPART_BUFFERED_UPLOAD_BYTESIZE_LIMIT'] || 16 * 1024 * 1024).to_i + def check_query_string(qs, sep) if qs if qs.bytesize > Rack::Utils.bytesize_limit diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index de39f95..b00484d 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -220,6 +220,110 @@ def rd.rewind; end wr.close end + should "reject excessive buffered mime data size in a single parameter" do + rd, wr = IO.pipe + def rd.rewind; end + wr.sync = true + + thr = Thread.new do + begin + wr.write("--AaB03x") + wr.write("\r\n") + wr.write('content-disposition: form-data; name="a"') + wr.write("\r\n") + wr.write("content-type: text/plain\r\n") + wr.write("\r\n") + wr.write("0" * 17 * 1024 * 1024) + wr.write("--AaB03x--\r\n") + wr.close + rescue Errno::EPIPE + # broken pipe is fine in case the server stops reading + end + true + end + + fixture = { + "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x", + :input => rd, + } + + env = Rack::MockRequest.env_for '/', fixture + lambda { + Rack::Multipart.parse_multipart(env) + }.should.raise(EOFError).message.should.equal "multipart data over retained size limit" + rd.close + + thr.value.should.equal true + wr.close + end + + should "reject excessive buffered mime data size when split into multiple parameters" do + rd, wr = IO.pipe + def rd.rewind; end + wr.sync = true + + thr = Thread.new do + 4.times do |i| + wr.write("\r\n--AaB03x") + wr.write("\r\n") + wr.write("content-disposition: form-data; name=\"a#{i}\"") + wr.write("\r\n") + wr.write("content-type: text/plain\r\n") + wr.write("\r\n") + wr.write("0" * 4 * 1024 * 1024) + end + wr.write("\r\n--AaB03x--\r\n") + wr.close + true + end + + fixture = { + "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x", + :input => rd, + } + + env = Rack::MockRequest.env_for '/', fixture + lambda { + Rack::Multipart.parse_multipart(env).keys + }.should.raise(EOFError).message.should.equal "multipart data over retained size limit" + rd.close + + thr.value.should.equal true + wr.close unless wr.closed? + end + + should "allow large nonbuffered mime parameters" do + rd, wr = IO.pipe + def rd.rewind; end + wr.sync = true + + thr = Thread.new do + wr.write("\r\n\r\n--AaB03x") + wr.write("\r\n") + wr.write('content-disposition: form-data; name="a"; filename="a.txt"') + wr.write("\r\n") + wr.write("content-type: text/plain\r\n") + wr.write("\r\n") + wr.write("0" * 16 * 1024 * 1024) + wr.write("\r\n--AaB03x--\r\n") + wr.close + true + end + + fixture = { + "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x", + "CONTENT_LENGTH" => (17 * 1024 * 1024).to_s, + :input => rd, + } + + env = Rack::MockRequest.env_for '/', fixture + Rack::Multipart.parse_multipart(env)['a'][:tempfile].read.bytesize.should.equal(16 * 1024 * 1024) + rd.close + + thr.value.should.equal true + wr.close unless wr.closed? + end + should "parse multipart upload with text file" do env = Rack::MockRequest.env_for("/", multipart_fixture(:text)) params = Rack::Multipart.parse_multipart(env) From 66864136c16b90829e142ea02740e9acae36c2d6 Mon Sep 17 00:00:00 2001 From: Tobias Kraze Date: Mon, 13 Oct 2025 14:06:10 +0200 Subject: [PATCH 5/6] rack: backport fix for CVE-2025-61780 but allow some safe X-Sendfile-Type headers --- lib/rack/sendfile.rb | 53 +++++++++++++++++++++++--- test/spec_sendfile.rb | 87 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 125 insertions(+), 15 deletions(-) diff --git a/lib/rack/sendfile.rb b/lib/rack/sendfile.rb index fbecbc2..ad40bda 100644 --- a/lib/rack/sendfile.rb +++ b/lib/rack/sendfile.rb @@ -41,18 +41,23 @@ module Rack # proxy_set_header X-Real-IP $remote_addr; # proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; # - # proxy_set_header X-Sendfile-Type X-Accel-Redirect; # proxy_set_header X-Accel-Mapping /var/www/=/files/; # # proxy_pass http://127.0.0.1:8080/; # } # - # Note that the X-Sendfile-Type header must be set exactly as shown above. # The X-Accel-Mapping header should specify the location on the file system, # followed by an equals sign (=), followed name of the private URL pattern # that it maps to. The middleware performs a simple substitution on the # resulting path. # + # # To enable X-Accel-Redirect, you must configure the middleware explicitly: + # + # use Rack::Sendfile, "X-Accel-Redirect" + # + # For security reasons, "X-Accel-Redirect" may not be set via the X-Sendfile-Type header. + # The sendfile variation must be set via the middleware constructor. + # # See Also: http://wiki.codemongers.com/NginxXSendfile # # === lighttpd @@ -97,15 +102,29 @@ module Rack # X-Accel-Mapping header. Mappings should be provided in tuples of internal to # external. The internal values may contain regular expression syntax, they # will be matched with case indifference. + # + # When X-Accel-Redirect is explicitly enabled via the variation parameter, + # and no application-level mappings are provided, the middleware will read + # the X-Accel-Mapping header from the proxy. This allows nginx to control + # the path mapping without requiring application-level configuration. + # + # === Security + # + # For security reasons, the X-Sendfile-Type header from HTTP requests may only + # be set to "X-Sendfile" or "X-Lighttpd-Send-File". Other values such as + # "X-Accel-Redirect" are not permitted to prevent information disclosure + # vulnerabilities where attackers could bypass proxy restrictions. + class Sendfile F = ::File + SAFE_SENDFILE_VARIATIONS = ['X-Sendfile', 'X-Lighttpd-Send-File'] def initialize(app, variation=nil, mappings=[]) @app = app @variation = variation @mappings = mappings.map do |internal, external| - [/^#{internal}/i, external] + [/\A#{internal}/i, external] end end @@ -142,19 +161,41 @@ def call(env) end private + + def x_sendfile_type(env) + sendfile_type = env['HTTP_X_SENDFILE_TYPE'] + if SAFE_SENDFILE_VARIATIONS.include?(sendfile_type) + sendfile_type + else + env['rack.errors'].puts "Unknown or unsafe x-sendfile variation: #{sendfile_type.inspect}" + end + end + def variation(env) @variation || env['sendfile.type'] || - env['HTTP_X_SENDFILE_TYPE'] + x_sendfile_type(env) + end + + def x_accel_mapping(env) + # Only allow header when: + # 1. X-Accel-Redirect is explicitly enabled via constructor. + # 2. No application-level mappings are configured. + return nil unless @variation == 'X-Accel-Redirect' + return nil if @mappings.any? + + env['HTTP_X_ACCEL_MAPPING'] end def map_accel_path(env, path) if mapping = @mappings.find { |internal,_| internal =~ path } path.sub(*mapping) - elsif mapping = env['HTTP_X_ACCEL_MAPPING'] + elsif mapping = x_accel_mapping(env) + # Safe to use header: explicit config + no app mappings internal, external = mapping.split('=', 2).map{ |p| p.strip } - path.sub(/^#{internal}/i, external) + path.sub(/\A#{internal}/i, external) end end + end end diff --git a/test/spec_sendfile.rb b/test/spec_sendfile.rb index affe430..b16a73d 100644 --- a/test/spec_sendfile.rb +++ b/test/spec_sendfile.rb @@ -22,12 +22,12 @@ def simple_app(body=sendfile_body) lambda { |env| [200, {'Content-Type' => 'text/plain'}, body] } end - def sendfile_app(body, mappings = []) - Rack::Lint.new Rack::Sendfile.new(simple_app(body), nil, mappings) + def sendfile_app(body, mappings = [], variation = nil) + Rack::Lint.new Rack::Sendfile.new(simple_app(body), variation, mappings) end - def request(headers={}, body=sendfile_body, mappings=[]) - yield Rack::MockRequest.new(sendfile_app(body, mappings)).get('/', headers) + def request(headers = {}, body = sendfile_body, mappings = [], variation = nil) + yield Rack::MockRequest.new(sendfile_app(body, mappings, variation)).get('/', headers) end def open_file(path) @@ -53,6 +53,18 @@ def open_file(path) response.body.should.equal 'Hello World' response.headers.should.not.include 'X-Sendfile' + io.rewind + io.read.should.equal "Unknown or unsafe x-sendfile variation: \"X-Banana\"\n" + end + end + + it "does nothing and logs to rack.errors when incorrect variation is configured" do + io = StringIO.new + request({ 'rack.errors' => io }, sendfile_body, [], 'X-Banana') do |response| + response.should.be.ok + response.body.should.equal 'Hello World' + response.headers.should.not.include 'X-Sendfile' + io.rewind io.read.should.equal "Unknown x-sendfile variation: \"X-Banana\"\n" end @@ -66,7 +78,7 @@ def open_file(path) response.headers.should.not.include 'X-Sendfile' io.rewind - io.read.should.equal "Unknown x-sendfile variation: \"Hello\\nCVE-2025-27111\"\n" + io.read.should.equal "Unknown or unsafe x-sendfile variation: \"Hello\\nCVE-2025-27111\"\n" end end @@ -88,12 +100,23 @@ def open_file(path) end end - it "sets X-Accel-Redirect response header and discards body" do + it "does not sets X-Accel-Redirect response header when it is set via X-Sendfile-Type" do headers = { 'HTTP_X_SENDFILE_TYPE' => 'X-Accel-Redirect', 'HTTP_X_ACCEL_MAPPING' => "#{Dir.tmpdir}/=/foo/bar/" } request headers do |response| + response.should.be.ok + response.body.should.equal 'Hello World' + response.headers.should.not.include 'X-Accel-Redirect' + end + end + + it "sets X-Accel-Redirect response header and discards body when set explicitly" do + headers = { + 'HTTP_X_ACCEL_MAPPING' => "#{Dir.tmpdir}/=/foo/bar/" + } + request(headers, sendfile_body, [], 'X-Accel-Redirect') do |response| response.should.be.ok response.body.should.be.empty response.headers['Content-Length'].should.equal '0' @@ -102,7 +125,7 @@ def open_file(path) end it 'writes to rack.error when no X-Accel-Mapping is specified' do - request 'HTTP_X_SENDFILE_TYPE' => 'X-Accel-Redirect' do |response| + request({}, sendfile_body, [], 'X-Accel-Redirect') do |response| response.should.be.ok response.body.should.equal 'Hello World' response.headers.should.not.include 'X-Accel-Redirect' @@ -133,14 +156,14 @@ def open_file(path) ["#{dir2}/", '/wibble/'] ] - request({'HTTP_X_SENDFILE_TYPE' => 'X-Accel-Redirect'}, first_body, mappings) do |response| + request({}, first_body, mappings, 'X-Accel-Redirect') do |response| response.should.be.ok response.body.should.be.empty response.headers['Content-Length'].should.equal '0' response.headers['X-Accel-Redirect'].should.equal '/foo/bar/rack_sendfile' end - request({'HTTP_X_SENDFILE_TYPE' => 'X-Accel-Redirect'}, second_body, mappings) do |response| + request({}, second_body, mappings, 'X-Accel-Redirect') do |response| response.should.be.ok response.body.should.be.empty response.headers['Content-Length'].should.equal '0' @@ -151,4 +174,50 @@ def open_file(path) FileUtils.remove_entry_secure dir2 end end + + it "ignores HTTP_X_ACCEL_MAPPING when application-level mappings are configured" do + # When app provides mappings, header should be ignored for security + begin + dir = Dir.mktmpdir + body = open_file(File.join(dir, 'rack_sendfile')) + body.puts 'test' + + app_mappings = [["#{dir}/", '/app/mapping/']] + app = Rack::Lint.new Rack::Sendfile.new(simple_app(body), "X-Accel-Redirect", app_mappings) + + headers = { + 'HTTP_X_ACCEL_MAPPING' => "#{dir}/=/attacker/path/" + } + + response = Rack::MockRequest.new(app).get('/', headers) + response.should.be.ok + response.body.should.be.empty + response.headers['x-accel-redirect'].should.equal '/app/mapping/rack_sendfile' + response.headers['x-accel-redirect'].should.not.equal '/attacker/path/rack_sendfile' + ensure + FileUtils.remove_entry_secure dir + end + end + + it "allows HTTP_X_ACCEL_MAPPING only when x-accel-redirect explicitly enabled with no app mappings" do + # This is the safe use case: explicit config + no app mappings = allow header + begin + dir = Dir.mktmpdir + body = open_file(File.join(dir, 'rack_sendfile')) + body.puts 'test' + + app = Rack::Lint.new Rack::Sendfile.new(simple_app(body), "X-Accel-Redirect", []) + + headers = { + 'HTTP_X_ACCEL_MAPPING' => "#{dir}/=/safe/nginx/mapping/" + } + + response = Rack::MockRequest.new(app).get('/', headers) + response.should.be.ok + response.body.should.be.empty + response.headers['x-accel-redirect'].should.equal '/safe/nginx/mapping/rack_sendfile' + ensure + FileUtils.remove_entry_secure dir + end + end end From 2f8358a005980abdfb125476001da109cef91848 Mon Sep 17 00:00:00 2001 From: Tobias Kraze Date: Mon, 13 Oct 2025 10:30:06 +0200 Subject: [PATCH 6/6] rack: backport fix for CVE-2025-61919 --- lib/rack/request.rb | 5 ++- lib/rack/utils.rb | 2 +- test/spec_request.rb | 87 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index 7ee9514..f8226b7 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -209,7 +209,10 @@ def POST @env["rack.request.form_hash"] elsif form_data? || parseable_data? unless @env["rack.request.form_hash"] = parse_multipart(env) - form_vars = @env["rack.input"].read + # Add 2 bytes. One to check whether it is over the limit, and a second + # in case the slice! call below removes the last byte + # If read returns nil, use the empty string + form_vars = @env["rack.input"].read(Rack::Utils.bytesize_limit + 2) || '' # Fix for Safari Ajax postings that always append \0 # form_vars.sub!(/\0\z/, '') # performance replacement: diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 8017505..3dcfac0 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -115,7 +115,7 @@ class << self def check_query_string(qs, sep) if qs if qs.bytesize > Rack::Utils.bytesize_limit - raise QueryLimitError, "total query size (#{qs.bytesize}) exceeds limit (#{Rack::Utils.bytesize_limit})" + raise QueryLimitError, "total query size exceeds limit (#{Rack::Utils.bytesize_limit})" end if (param_count = qs.count(sep.is_a?(String) ? sep : '&')) >= Rack::Utils.params_limit diff --git a/test/spec_request.rb b/test/spec_request.rb index fcccb10..17ddc75 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -290,6 +290,93 @@ req.POST.should.equal "foo" => "bar", "quux" => "bla" end + should "limit POST body read to bytesize_limit when parsing url-encoded data" do + # Create a mock input that tracks read calls + reads = [] + mock_input = Object.new + mock_input.define_singleton_method(:read) do |len=nil| + reads << len + # Return mutable string + "foo=bar".dup + end + mock_input.define_singleton_method(:rewind) do + # no-op for compatibility + end + + request = Rack::Request.new \ + Rack::MockRequest.env_for("/", + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'application/x-www-form-urlencoded', + :input => mock_input) + + request.POST.should.equal "foo" => "bar" + + # Verify read was called with a limit (bytesize_limit + 2), not nil + reads.size.should.equal 1 + reads.first.should.not.be.nil + reads.first.should.equal(Rack::Utils.bytesize_limit + 2) + end + + should "handle nil return from rack.input.read when parsing url-encoded data" do + # Simulate an input that returns nil on read + mock_input = Object.new + mock_input.define_singleton_method(:read) do |len=nil| + nil + end + mock_input.define_singleton_method(:rewind) do + # no-op for compatibility + end + + request = Rack::Request.new \ + Rack::MockRequest.env_for("/", + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'application/x-www-form-urlencoded', + :input => mock_input) + + # Should handle nil gracefully and return empty hash + request.POST.should.equal({}) + end + + should "truncate POST body at bytesize_limit when parsing url-encoded data" do + # Create input larger than limit + large_body = "a=1&" * 1000000 # Very large body + + request = Rack::Request.new \ + Rack::MockRequest.env_for("/", + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'application/x-www-form-urlencoded', + :input => large_body) + + # Should parse only up to the limit without reading entire body into memory + # The actual parsing may fail due to size limit, which is expected + proc { request.POST }.should.raise Rack::Utils::QueryLimitError + end + + should "clean up Safari's ajax POST body with limited read" do + # Verify Safari null-byte cleanup still works with bounded read + reads = [] + mock_input = Object.new + mock_input.define_singleton_method(:read) do |len=nil| + reads << len + # Return mutable string (dup ensures it's not frozen) + "foo=bar\0".dup + end + mock_input.define_singleton_method(:rewind) do + # no-op for compatibility + end + + request = Rack::Request.new \ + Rack::MockRequest.env_for("/", + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'application/x-www-form-urlencoded', + :input => mock_input) + + request.POST.should.equal "foo" => "bar" + + # Verify bounded read was used + reads.first.should.not.be.nil + end + should "get value by key from params with #[]" do req = Rack::Request.new \ Rack::MockRequest.env_for("?foo=quux")