Escape untrusted text when logging

This fixes a shell escape issue [CVE-2022-30123]
rack · May 27, 2022 · b426cc2 · b426cc2
1 parent d286516
commit b426cc2
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 1 deletion.
diff --git a/lib/rack/common_logger.rb b/lib/rack/common_logger.rb
@@ -65,6 +65,8 @@ def log(env, status, response_headers, began_at)
         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
@@ -347,7 +347,7 @@ def check_environment(env)
 
         ## * 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
@@ -25,6 +25,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("/")
@@ -103,6 +107,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 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
@@ -212,6 +212,11 @@ def obj.fatal(*) end
     }.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).