CVE-2021-43840: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: message_bus is a messaging bus for Ruby processes and web clients. In versions prior to 3.3.7 users who deployed message bus with diagnostics features enabled (default off) are vulnerable to a path traversal bug, which could lead to disclosure of secret information on a machine if an unintended user were to gain access to the diagnostic route. The impact is also greater if there is no proxy for your web application as the number of steps up the directories is not bounded. For deployments which uses a proxy, the impact varies. For example, If a request goes through a proxy like Nginx with `merge_slashes` enabled, the number of steps up the directories that can be read is limited to 3 levels. This issue has been patched in version 3.3.7. Users unable to upgrade should ensure that MessageBus::Diagnostics is disabled.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 6.5
Published: 2021-12-17T19:15Z
Repository: https://github.com/discourse/message_bus
Language: Ruby
Files changed: CHANGELOG, lib/message_bus/rack/diagnostics.rb, spec/lib/message_bus/rack/middleware_spec.rb
Commit date: 2021-12-17 14:11:53 +0800
Commit message: SECURITY: Fix path traversal on diagnostics route.

Diff: diff --git a/CHANGELOG b/CHANGELOG
index bc17bca..fcc4092 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,8 +1,9 @@
 Unreleased
 
   - FIX: Prevent simple polling from clobbering the session
+  - SECURITY: Fix path traversal on diagnostics route.
 
 31-05-2021
 
 - Version 3.3.6
 
diff --git a/lib/message_bus/rack/diagnostics.rb b/lib/message_bus/rack/diagnostics.rb
index 893c867..a66efb3 100644
--- a/lib/message_bus/rack/diagnostics.rb
+++ b/lib/message_bus/rack/diagnostics.rb
@@ -11,10 +11,19 @@ class MessageBus::Rack::Diagnostics
   def initialize(app, config = {})
     @app = app
     @bus = config[:message_bus] || MessageBus
   end
 
+  JS_ASSETS = %w{
+    jquery-1.8.2.js
+    react.js
+    react-dom.js
+    babel.min.js
+    message-bus.js
+    application.jsx
+  }
+
   # Process an HTTP request from a subscriber client
   # @param [Rack::Request::Env] env the request environment
   def call(env)
     return @app.call(env) unless env['PATH_INFO'].start_with? "#{@bus.base_route}message-bus/_diagnostics"
 
@@ -37,11 +46,12 @@ class MessageBus::Rack::Diagnostics
       @bus.publish('/_diagnostics/hup', hostname: hostname, pid: pid.to_i)
       return [200, {}, ['ok']]
     end
 
     asset = route.split('/assets/')[1]
-    if asset && !asset !~ /\//
+
+    if asset && JS_ASSETS.include?(asset)
       content = asset_contents(asset)
       return [200, { 'Content-Type' => 'application/javascript;charset=UTF-8' }, [content]]
     end
 
     [404, {}, ['not found']]
@@ -72,24 +82,37 @@ class MessageBus::Rack::Diagnostics
 
   def asset_path(asset)
     File.expand_path("../../../../assets/#{asset}", __FILE__)
   end
 
+  def script_tags
+    tags = []
+
+    JS_ASSETS.each do |asset|
+      type =
+        if asset.end_with?('.js')
+          'text/javascript'
+        elsif asset.end_with?('.jsx')
+          'text/jsx'
+        end
+
+      tags << js_asset(asset, type)
+    end
+
+    tags.join("\n")
+  end
+
   def index
     html = <<~HTML
       <!DOCTYPE html>
       <html>
         <head>
         </head>
         <body>
           <div id="app"></div>
-          #{js_asset "jquery-1.8.2.js"}
-          #{js_asset "react.js"}
-          #{js_asset "react-dom.js"}
-          #{js_asset "babel.min.js"}
-          #{js_asset "message-bus.js"}
-          #{js_asset "application.jsx", "text/jsx"}
+
+          #{script_tags}
         </body>
       </html>
     HTML
 
     [200, { "content-type" => "text/html;" }, [html]]
diff --git a/spec/lib/message_bus/rack/middleware_spec.rb b/spec/lib/message_bus/rack/middleware_spec.rb
index 1525a65..3fb5442 100644
--- a/spec/lib/message_bus/rack/middleware_spec.rb
+++ b/spec/lib/message_bus/rack/middleware_spec.rb
@@ -140,17 +140,16 @@ describe MessageBus::Rack::Middleware do
       middleware.started_listener.must_equal false
     end
   end
 
   describe "diagnostics" do
-    it "should return a 403 if a user attempts to get at the _diagnostics path" do
+    it "should return a 403 if an unauthorized user attempts to get at the _diagnostics path" do
       get "/message-bus/_diagnostics"
       last_response.status.must_equal 403
     end
 
     it "should get a 200 with html for an authorized user" do
-
       def @bus.is_admin_lookup
         proc { |_| true }
       end
 
       get "/message-bus/_diagnostics"
@@ -169,19 +168,27 @@ describe MessageBus::Rack::Middleware do
         last_response.status.must_equal 200
       end
     end
 
     it "should get the script it asks for" do
-
       def @bus.is_admin_lookup
         proc { |_| true }
       end
 
       get "/message-bus/_diagnostics/assets/message-bus.js"
       last_response.status.must_equal 200
       last_response.content_type.must_equal "application/javascript;charset=UTF-8"
     end
+
+    it "should return 404 for invalid assets path" do
+      def @bus.is_admin_lookup
+        proc { |_| true }
+      end
+
+      get "/message-bus/_diagnostics/assets/../Gemfile"
+      last_response.status.must_equal 404
+    end
   end
 
   describe "polling" do
     before do
       @bus.long_polling_enabled = false

Vulnerable code:     if asset && !asset !~ /\//
          #{js_asset "jquery-1.8.2.js"}
          #{js_asset "react.js"}
          #{js_asset "react-dom.js"}
          #{js_asset "babel.min.js"}
          #{js_asset "message-bus.js"}
          #{js_asset "application.jsx", "text/jsx"}
    it "should return a 403 if a user attempts to get at the _diagnostics path" do


Fixed code:   - SECURITY: Fix path traversal on diagnostics route.
  JS_ASSETS = %w{
    jquery-1.8.2.js
    react.js
    react-dom.js
    babel.min.js
    message-bus.js
    application.jsx
  }


    if asset && JS_ASSETS.include?(asset)
  def script_tags
    tags = []

    JS_ASSETS.each do |asset|
      type =
        if asset.end_with?('.js')
          'text/javascript'
        elsif asset.end_with?('.jsx')
          'text/jsx'
        end

      tags << js_asset(asset, type)
    end

    tags.join("\n")
  end


          #{script_tags}
    it "should return a 403 if an unauthorized user attempts to get at the _diagnostics path" do

    it "should return 404 for invalid assets path" do
      def @bus.is_admin_lookup
        proc { |_| true }
      end

      get "/message-bus/_diagnostics/assets/../Gemfile"
      last_response.status.must_equal 404
    end