Backport of: From 3f5a4249118d09d199fe480466c8c6717e43b6e3 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Tue, 6 May 2025 19:08:08 +0900 Subject: [PATCH] Merge commit from fork * Apply bytesize and number of param limits in QueryParser The param limit is 4096, chosen because it matches the existing multipart limit. The bytesize limit is 4MB. These limits should substantially exceed what almost all applications need, though there will likely be applications that require higher limits. Allow overriding the limits on a per-QueryParser basis via the constructors, and allow overriding the default limits with environment variables RACK_QUERY_PARSER_BYTESIZE_LIMIT and RACK_QUERY_PARSER_PARAMS_LIMIT. Add new Rack::QueryParser::QueryLimitError to raise in case one of the limits are exceeded, and make ParamsTooDeepError an alias to, since that is also a case where a limit is exceeded. This allows code that already rescues ParamsTooDeepError to automatically handle these other limits as well. * Update CHANGELOG. --------- Co-authored-by: Samuel Williams --- CHANGELOG.md | 1 + README.rdoc | 27 +++++++++++++++++ lib/rack/query_parser.rb | 63 ++++++++++++++++++++++++++++++++------- test/spec_query_parser.rb | 33 ++++++++++++++++++++ test/spec_request.rb | 8 +++++ 5 files changed, 122 insertions(+), 10 deletions(-) create mode 100644 test/spec_query_parser.rb --- a/lib/rack/query_parser.rb +++ b/lib/rack/query_parser.rb @@ -18,16 +18,47 @@ # sequence. class InvalidParameterError < ArgumentError; end - def self.make_default(key_space_limit, param_depth_limit) - new Params, key_space_limit, param_depth_limit + # QueryLimitError is for errors raised when the query provided exceeds one + # of the query parser limits. + class QueryLimitError < RangeError + end + + # ParamsTooDeepError is the old name for the error that is raised when params + # are recursively nested over the specified limit. Make it the same as + # as QueryLimitError, so that code that rescues ParamsTooDeepError error + # to handle bad query strings also now handles other limits. + ParamsTooDeepError = QueryLimitError + + def self.make_default(key_space_limit, param_depth_limit, **options) + new(Params, key_space_limit, param_depth_limit, **options) end attr_reader :key_space_limit, :param_depth_limit - def initialize(params_class, key_space_limit, param_depth_limit) + env_int = lambda do |key, val| + if str_val = ENV[key] + begin + val = Integer(str_val, 10) + rescue ArgumentError + raise ArgumentError, "non-integer value provided for environment variable #{key}" + end + end + + val + end + + BYTESIZE_LIMIT = env_int.call("RACK_QUERY_PARSER_BYTESIZE_LIMIT", 4194304) + private_constant :BYTESIZE_LIMIT + + PARAMS_LIMIT = env_int.call("RACK_QUERY_PARSER_PARAMS_LIMIT", 4096) + private_constant :PARAMS_LIMIT + + def initialize(params_class, key_space_limit, param_depth_limit, bytesize_limit: BYTESIZE_LIMIT, params_limit: PARAMS_LIMIT) @params_class = params_class @key_space_limit = key_space_limit @param_depth_limit = param_depth_limit + @bytesize_limit = bytesize_limit + @params_limit = params_limit end # Stolen from Mongrel, with some small modifications: @@ -40,7 +71,7 @@ params = make_params - (qs || '').split(d ? (COMMON_SEP[d] || /[#{d}] */n) : DEFAULT_SEP).each do |p| + check_query_string(qs, d).split(d ? (COMMON_SEP[d] || /[#{d}] */n) : DEFAULT_SEP).each do |p| next if p.empty? k, v = p.split('=', 2).map!(&unescaper) @@ -67,7 +98,7 @@ return {} if qs.nil? || qs.empty? params = make_params - qs.split(d ? (COMMON_SEP[d] || /[#{d}] */n) : DEFAULT_SEP).each do |p| + check_query_string(qs, d).split(d ? (COMMON_SEP[d] || /[#{d}] */n) : DEFAULT_SEP).each do |p| k, v = p.split('=', 2).map! { |s| unescape(s) } normalize_params(params, k, v, param_depth_limit) @@ -152,8 +183,24 @@ true end - def unescape(s) - Utils.unescape(s) + def check_query_string(qs, sep) + if qs + if qs.bytesize > @bytesize_limit + raise QueryLimitError, "total query size (#{qs.bytesize}) exceeds limit (#{@bytesize_limit})" + end + + if (param_count = qs.count(sep.is_a?(String) ? sep : '&')) >= @params_limit + raise QueryLimitError, "total number of query parameters (#{param_count+1}) exceeds limit (#{@params_limit})" + end + + qs + else + '' + end + end + + def unescape(string, encoding = Encoding::UTF_8) + Utils.unescape(string, encoding) end class Params --- /dev/null +++ b/test/spec_query_parser.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require_relative 'helper' +require_relative '../lib/rack/query_parser' + +describe Rack::QueryParser do + it "can normalize values with missing values" do + query_parser = Rack::QueryParser.make_default(Rack::Utils.key_space_limit, 8) + query_parser.parse_nested_query("a=a").must_equal({"a" => "a"}) + query_parser.parse_nested_query("a=").must_equal({"a" => ""}) + query_parser.parse_nested_query("a").must_equal({"a" => nil}) + end + + it "accepts bytesize_limit to specify maximum size of query string to parse" do + query_parser = Rack::QueryParser.make_default(Rack::Utils.key_space_limit, 32, bytesize_limit: 3) + query_parser.parse_query("a=a").must_equal({"a" => "a"}) + query_parser.parse_nested_query("a=a").must_equal({"a" => "a"}) + query_parser.parse_nested_query("a=a", '&').must_equal({"a" => "a"}) + proc { query_parser.parse_query("a=aa") }.must_raise Rack::QueryParser::QueryLimitError + proc { query_parser.parse_nested_query("a=aa") }.must_raise Rack::QueryParser::QueryLimitError + proc { query_parser.parse_nested_query("a=aa", '&') }.must_raise Rack::QueryParser::QueryLimitError + end + + it "accepts params_limit to specify maximum number of query parameters to parse" do + query_parser = Rack::QueryParser.make_default(Rack::Utils.key_space_limit, 32, params_limit: 2) + query_parser.parse_query("a=a&b=b").must_equal({"a" => "a", "b" => "b"}) + query_parser.parse_nested_query("a=a&b=b").must_equal({"a" => "a", "b" => "b"}) + query_parser.parse_nested_query("a=a&b=b", '&').must_equal({"a" => "a", "b" => "b"}) + proc { query_parser.parse_query("a=a&b=b&c=c") }.must_raise Rack::QueryParser::QueryLimitError + proc { query_parser.parse_nested_query("a=a&b=b&c=c", '&') }.must_raise Rack::QueryParser::QueryLimitError + proc { query_parser.parse_query("b[]=a&b[]=b&b[]=c") }.must_raise Rack::QueryParser::QueryLimitError + end +end --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -1454,6 +1454,10 @@ class NonDelegate < Rack::Request def delegate?; false; end + + def query_parser + Rack::QueryParser.make_default(Rack::Utils.key_space_limit, Rack::Utils.param_depth_limit, bytesize_limit: 2**30) + end end def make_request(env) @@ -1477,6 +1481,10 @@ def delegate?; true; end def env; @req.env.dup; end + + def query_parser + Rack::QueryParser.make_default(Rack::Utils.key_space_limit, Rack::Utils.param_depth_limit, bytesize_limit: 2**30) + end end def make_request(env)