diff options
| author | Aaron Patterson <tenderlove@ruby-lang.org> | 2022-05-26 16:18:10 -0700 |
|---|---|---|
| committer | Aaron Patterson <tenderlove@ruby-lang.org> | 2022-05-26 16:26:34 -0700 |
| commit | 434413f5cff1ca36d70c3e5a3544c5c2d7b636bf (patch) | |
| tree | 365f37b320309fc473d4fc5fd088a955f1f55ce4 | |
| parent | a872bcce127a6f4a0f16af8f28b686955ba32814 (diff) | |
| download | rack-434413f5cff1ca36d70c3e5a3544c5c2d7b636bf.tar.gz | |
Escape untrusted text when logging
This fixes a shell escape issue
[CVE-2022-30123]
| -rw-r--r-- | lib/rack/common_logger.rb | 3 | ||||
| -rw-r--r-- | lib/rack/lint.rb | 2 | ||||
| -rw-r--r-- | test/spec_common_logger.rb | 12 | ||||
| -rw-r--r-- | test/spec_lint.rb | 5 |
4 files changed, 21 insertions, 1 deletions
diff --git a/lib/rack/common_logger.rb b/lib/rack/common_logger.rb index 3810b269..9c6f9214 100644 --- a/lib/rack/common_logger.rb +++ b/lib/rack/common_logger.rb @@ -60,7 +60,10 @@ module Rack 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 16b5feea..a157aae2 100644 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -337,7 +337,7 @@ module Rack check_hijack env ## * The <tt>REQUEST_METHOD</tt> 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/test/spec_common_logger.rb b/test/spec_common_logger.rb index dd55c2f8..4ddb5f03 100644 --- a/test/spec_common_logger.rb +++ b/test/spec_common_logger.rb @@ -19,6 +19,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("/") @@ -83,6 +87,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/) + 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 7b2151b0..d3d72418 100644 --- a/test/spec_lint.rb +++ b/test/spec_lint.rb @@ -164,6 +164,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/) |
