Skip to content

Commit b426cc2

Browse files
committed
Escape untrusted text when logging
This fixes a shell escape issue [CVE-2022-30123]
1 parent d286516 commit b426cc2

4 files changed

Lines changed: 20 additions & 1 deletion

File tree

lib/rack/common_logger.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ def log(env, status, response_headers, began_at)
6565
length,
6666
Utils.clock_time - began_at)
6767

68+
msg.gsub!(/[^[:print:]\n]/) { |c| "\\x#{c.ord}" }
69+
6870
logger = @logger || request.get_header(RACK_ERRORS)
6971
# Standard library logger doesn't support write but it supports << which actually
7072
# calls to write on the log device without formatting

lib/rack/lint.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ def check_environment(env)
347347

348348
## * The <tt>REQUEST_METHOD</tt> must be a valid token.
349349
unless env[REQUEST_METHOD] =~ /\A[0-9A-Za-z!\#$%&'*+.^_`|~-]+\z/
350-
raise LintError, "REQUEST_METHOD unknown: #{env[REQUEST_METHOD]}"
350+
raise LintError, "REQUEST_METHOD unknown: #{env[REQUEST_METHOD].dump}"
351351
end
352352

353353
## * The <tt>SCRIPT_NAME</tt>, if non-empty, must start with <tt>/</tt>

test/spec_common_logger.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
[200,
2626
{ "content-type" => "text/html", "content-length" => "0" },
2727
[]]}
28+
app_without_lint = lambda { |env|
29+
[200,
30+
{ "content-type" => "text/html", "content-length" => length.to_s },
31+
[obj]]}
2832

2933
it "log to rack.errors by default" do
3034
res = Rack::MockRequest.new(Rack::CommonLogger.new(app)).get("/")
@@ -103,6 +107,14 @@ def with_mock_time(t = 0)
103107
(0..1).must_include duration.to_f
104108
end
105109

110+
it "escapes non printable characters except newline" do
111+
logdev = StringIO.new
112+
log = Logger.new(logdev)
113+
Rack::MockRequest.new(Rack::CommonLogger.new(app_without_lint, log)).request("GET\b", "/hello")
114+
115+
logdev.string.must_match(/GET\\x8 \/hello HTTP\/1\.1/)
116+
end
117+
106118
it "log path with PATH_INFO" do
107119
logdev = StringIO.new
108120
log = Logger.new(logdev)

test/spec_lint.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,11 @@ def obj.fatal(*) end
212212
}.must_raise(Rack::Lint::LintError).
213213
message.must_match(/REQUEST_METHOD/)
214214

215+
lambda {
216+
Rack::Lint.new(nil).call(env("REQUEST_METHOD" => "OOPS?\b!"))
217+
}.must_raise(Rack::Lint::LintError).
218+
message.must_match(/OOPS\?\\/)
219+
215220
lambda {
216221
Rack::Lint.new(nil).call(env("SCRIPT_NAME" => "howdy"))
217222
}.must_raise(Rack::Lint::LintError).

0 commit comments

Comments
 (0)