Skip to content

Commit

Permalink
Escape untrusted text when logging
Browse files Browse the repository at this point in the history
This fixes a shell escape issue

[CVE-2022-30123]
  • Loading branch information
tenderlove committed May 26, 2022
1 parent ad699ca commit 817e695
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 1 deletion.
3 changes: 3 additions & 0 deletions lib/rack/common_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ def log(env, status, header, began_at)
length,
now - 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)
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/lint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ def check_env(env)
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/
}

Expand Down
12 changes: 12 additions & 0 deletions test/spec_common_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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("/")
Expand Down Expand Up @@ -85,6 +89,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

def length
123
end
Expand Down
5 changes: 5 additions & 0 deletions test/spec_lint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ def env(*args)
}.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).
Expand Down

0 comments on commit 817e695

Please sign in to comment.