diff options
author | Aaron Patterson <tenderlove@ruby-lang.org> | 2022-05-26 16:18:10 -0700 |
---|---|---|
committer | Aaron Patterson <aaron.patterson@gmail.com> | 2022-05-27 08:41:41 -0700 |
commit | b426cc224908ec6ed6eb8729325392b048215d88 (patch) | |
tree | 5e24cac509cd2f702ea0a83e013eb100ae9c6075 | |
parent | d286516cbd58fbb2ad6944ce9040e9ba96d9371a (diff) | |
download | rack-b426cc224908ec6ed6eb8729325392b048215d88.tar.gz |
Escape untrusted text when logging
This fixes a shell escape issue [CVE-2022-30123]
-rw-r--r-- | lib/rack/common_logger.rb | 2 | ||||
-rwxr-xr-x | lib/rack/lint.rb | 2 | ||||
-rw-r--r-- | test/spec_common_logger.rb | 12 | ||||
-rwxr-xr-x | test/spec_lint.rb | 5 |
4 files changed, 20 insertions, 1 deletions
diff --git a/lib/rack/common_logger.rb b/lib/rack/common_logger.rb index 53f42cc5..6235cef2 100644 --- a/lib/rack/common_logger.rb +++ b/lib/rack/common_logger.rb @@ -65,6 +65,8 @@ module Rack length, Utils.clock_time - began_at) + msg.gsub!(/[^[:print:]\n]/) { |c| "\\x#{c.ord}" } + logger = @logger || request.get_header(RACK_ERRORS) # Standard library logger doesn't support write but it supports << which actually # calls to write on the log device without formatting diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index f451302c..a4b34196 100755 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -347,7 +347,7 @@ module Rack ## * The <tt>REQUEST_METHOD</tt> must be a valid token. unless env[REQUEST_METHOD] =~ /\A[0-9A-Za-z!\#$%&'*+.^_`|~-]+\z/ - raise LintError, "REQUEST_METHOD unknown: #{env[REQUEST_METHOD]}" + raise LintError, "REQUEST_METHOD unknown: #{env[REQUEST_METHOD].dump}" end ## * The <tt>SCRIPT_NAME</tt>, if non-empty, must start with <tt>/</tt> diff --git a/test/spec_common_logger.rb b/test/spec_common_logger.rb index 56495e6c..f5f182aa 100644 --- a/test/spec_common_logger.rb +++ b/test/spec_common_logger.rb @@ -25,6 +25,10 @@ describe Rack::CommonLogger do [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("/") @@ -103,6 +107,14 @@ describe Rack::CommonLogger do (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 HTTP\/1\.1/) + 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 4a9eac26..2242b1f9 100755 --- a/test/spec_lint.rb +++ b/test/spec_lint.rb @@ -213,6 +213,11 @@ describe Rack::Lint do 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). message.must_match(/must start with/) |