summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAaron Patterson <tenderlove@ruby-lang.org>2022-05-26 16:18:10 -0700
committerAaron Patterson <tenderlove@ruby-lang.org>2022-05-26 16:26:34 -0700
commit434413f5cff1ca36d70c3e5a3544c5c2d7b636bf (patch)
tree365f37b320309fc473d4fc5fd088a955f1f55ce4
parenta872bcce127a6f4a0f16af8f28b686955ba32814 (diff)
downloadrack-434413f5cff1ca36d70c3e5a3544c5c2d7b636bf.tar.gz
Escape untrusted text when logging
This fixes a shell escape issue [CVE-2022-30123]
-rw-r--r--lib/rack/common_logger.rb3
-rw-r--r--lib/rack/lint.rb2
-rw-r--r--test/spec_common_logger.rb12
-rw-r--r--test/spec_lint.rb5
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/)