diff --git a/CHANGELOG.md b/CHANGELOG.md index e74cca0e1..80f57c99f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to this project will be documented in this file. For info on how to format all future additions to this file please reference [Keep A Changelog](https://keepachangelog.com/en/1.0.0/). +## [2.2.3.1] - 2022-05-27 + +- [CVE-2022-30123] Fix shell escaping issue in Common Logger +- [CVE-2022-30122] Restrict parsing of broken MIME attachments + ## [2.2.3] - 2020-02-11 - [CVE-2020-8184] Only decode cookie values diff --git a/lib/rack/common_logger.rb b/lib/rack/common_logger.rb index 3810b2693..9c6f92147 100644 --- a/lib/rack/common_logger.rb +++ b/lib/rack/common_logger.rb @@ -60,7 +60,10 @@ def log(env, status, header, began_at) length, Utils.clock_time - began_at ] + msg.gsub!(/[^[:print:]\n]/) { |c| "\\x#{c.ord}" } + logger = @logger || env[RACK_ERRORS] + # Standard library logger doesn't support write but it supports << which actually # calls to write on the log device without formatting if logger.respond_to?(:write) diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index 16b5feea2..a157aae23 100644 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -337,7 +337,7 @@ def check_env(env) check_hijack env ## * The REQUEST_METHOD must be a valid token. - assert("REQUEST_METHOD unknown: #{env[REQUEST_METHOD]}") { + assert("REQUEST_METHOD unknown: #{env[REQUEST_METHOD].dump}") { env[REQUEST_METHOD] =~ /\A[0-9A-Za-z!\#$%&'*+.^_`|~-]+\z/ } diff --git a/lib/rack/multipart.rb b/lib/rack/multipart.rb index 45f43bb68..10f8e5fa3 100644 --- a/lib/rack/multipart.rb +++ b/lib/rack/multipart.rb @@ -16,8 +16,7 @@ module Multipart TOKEN = /[^\s()<>,;:\\"\/\[\]?=]+/ CONDISP = /Content-Disposition:\s*#{TOKEN}\s*/i VALUE = /"(?:\\"|[^"])*"|#{TOKEN}/ - BROKEN_QUOTED = /^#{CONDISP}.*;\s*filename="(.*?)"(?:\s*$|\s*;\s*#{TOKEN}=)/i - BROKEN_UNQUOTED = /^#{CONDISP}.*;\s*filename=(#{TOKEN})/i + BROKEN = /^#{CONDISP}.*;\s*filename=(#{VALUE})/i MULTIPART_CONTENT_TYPE = /Content-Type: (.*)#{EOL}/ni MULTIPART_CONTENT_DISPOSITION = /Content-Disposition:.*;\s*name=(#{VALUE})/ni MULTIPART_CONTENT_ID = /Content-ID:\s*([^#{EOL}]*)/ni diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index 2eb38380b..e8ed3e976 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -301,8 +301,9 @@ def get_filename(head) elsif filename = params['filename*'] encoding, _, filename = filename.split("'", 3) end - when BROKEN_QUOTED, BROKEN_UNQUOTED + when BROKEN filename = $1 + filename = $1 if filename =~ /^"(.*)"$/ end return unless filename diff --git a/lib/rack/version.rb b/lib/rack/version.rb index aad9c5915..233e16223 100644 --- a/lib/rack/version.rb +++ b/lib/rack/version.rb @@ -20,7 +20,7 @@ def self.version VERSION.join(".") end - RELEASE = "2.2.3" + RELEASE = "2.2.3.1" # Return the Rack release as a dotted string. def self.release diff --git a/test/multipart/filename_with_escaped_quotes_and_modification_param b/test/multipart/filename_with_escaped_quotes_and_modification_param index 7619bd507..929f6ad3f 100644 --- a/test/multipart/filename_with_escaped_quotes_and_modification_param +++ b/test/multipart/filename_with_escaped_quotes_and_modification_param @@ -1,6 +1,6 @@ --AaB03x Content-Type: image/jpeg -Content-Disposition: attachment; name="files"; filename=""human" genome.jpeg"; modification-date="Wed, 12 Feb 1997 16:29:51 -0500"; +Content-Disposition: attachment; name="files"; filename="\"human\" genome.jpeg"; modification-date="Wed, 12 Feb 1997 16:29:51 -0500"; Content-Description: a complete map of the human genome contents diff --git a/test/spec_common_logger.rb b/test/spec_common_logger.rb index dd55c2f8b..4ddb5f03d 100644 --- a/test/spec_common_logger.rb +++ b/test/spec_common_logger.rb @@ -19,6 +19,10 @@ [200, { "Content-Type" => "text/html", "Content-Length" => "0" }, []]} + app_without_lint = lambda { |env| + [200, + { "content-type" => "text/html", "content-length" => length.to_s }, + [obj]]} it "log to rack.errors by default" do res = Rack::MockRequest.new(Rack::CommonLogger.new(app)).get("/") @@ -83,6 +87,14 @@ def with_mock_time(t = 0) (0..1).must_include duration.to_f end + it "escapes non printable characters except newline" do + logdev = StringIO.new + log = Logger.new(logdev) + Rack::MockRequest.new(Rack::CommonLogger.new(app_without_lint, log)).request("GET\b", "/hello") + + logdev.string.must_match(/GET\\x8 \/hello/) + end + it "log path with PATH_INFO" do logdev = StringIO.new log = Logger.new(logdev) diff --git a/test/spec_lint.rb b/test/spec_lint.rb index 7b2151b02..d3d724180 100644 --- a/test/spec_lint.rb +++ b/test/spec_lint.rb @@ -163,6 +163,11 @@ def obj.error(*) end }.must_raise(Rack::Lint::LintError). message.must_match(/REQUEST_METHOD/) + lambda { + Rack::Lint.new(nil).call(env("REQUEST_METHOD" => "OOPS?\b!")) + }.must_raise(Rack::Lint::LintError). + message.must_match(/OOPS\?\\/) + lambda { Rack::Lint.new(nil).call(env("SCRIPT_NAME" => "howdy")) }.must_raise(Rack::Lint::LintError). diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index 717e0dc81..0275fa52d 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -433,19 +433,6 @@ def initialize(*) params["files"][:tempfile].read.must_equal "contents" end - it "parse filename with unescaped quotes" do - env = Rack::MockRequest.env_for("/", multipart_fixture(:filename_with_unescaped_quotes)) - params = Rack::Multipart.parse_multipart(env) - params["files"][:type].must_equal "application/octet-stream" - params["files"][:filename].must_equal "escape \"quotes" - params["files"][:head].must_equal "Content-Disposition: form-data; " + - "name=\"files\"; " + - "filename=\"escape \"quotes\"\r\n" + - "Content-Type: application/octet-stream\r\n" - params["files"][:name].must_equal "files" - params["files"][:tempfile].read.must_equal "contents" - end - it "parse filename with escaped quotes and modification param" do env = Rack::MockRequest.env_for("/", multipart_fixture(:filename_with_escaped_quotes_and_modification_param)) params = Rack::Multipart.parse_multipart(env) @@ -454,7 +441,7 @@ def initialize(*) params["files"][:head].must_equal "Content-Type: image/jpeg\r\n" + "Content-Disposition: attachment; " + "name=\"files\"; " + - "filename=\"\"human\" genome.jpeg\"; " + + "filename=\"\\\"human\\\" genome.jpeg\"; " + "modification-date=\"Wed, 12 Feb 1997 16:29:51 -0500\";\r\n" + "Content-Description: a complete map of the human genome\r\n" params["files"][:name].must_equal "files"