Fix uncaught query param encoding errors (#12741)

diff --git a/app/middleware/handle_bad_encoding_middleware.rb b/app/middleware/handle_bad_encoding_middleware.rb
new file mode 100644 (file)
index 0000000..6fce84b
--- /dev/null
+++ b/app/middleware/handle_bad_encoding_middleware.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+# See: https://jamescrisp.org/2018/05/28/fixing-invalid-query-parameters-invalid-encoding-in-a-rails-app/
+
+class HandleBadEncodingMiddleware
+  def initialize(app)
+    @app = app
+  end
+
+  def call(env)
+    begin
+      Rack::Utils.parse_nested_query(env['QUERY_STRING'].to_s)
+    rescue Rack::Utils::InvalidParameterError
+      env['QUERY_STRING'] = ''
+    end
+
+    @app.call(env)
+  end
+end

diff --git a/config/application.rb b/config/application.rb
index e1f7ae707daff9f3b24db0c48515f9c47ab14040..58e59fd51b054ee57847f4dc59092bd0ac5952a8 100644 (file)
--- a/config/application.rb
+++ b/config/application.rb
@@ -7,6 +7,7 @@ require 'rails/all'
 Bundler.require(*Rails.groups)
 
 require_relative '../app/lib/exceptions'
+require_relative '../app/middleware/handle_bad_encoding_middleware'
 require_relative '../lib/paperclip/lazy_thumbnail'
 require_relative '../lib/paperclip/gif_transcoder'
 require_relative '../lib/paperclip/video_transcoder'
@@ -118,6 +119,7 @@ module Mastodon
 
     config.active_job.queue_adapter = :sidekiq
 
+    config.middleware.insert_before Rack::Runtime, HandleBadEncodingMiddleware
     config.middleware.use Rack::Attack
     config.middleware.use Rack::Deflater
 

diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb
index 273cac9ca3d437c3108deeaf75a38095cf214d39..3cd7ea3a6b1a8277aeb98d08022f948fd8030c21 100644 (file)
--- a/config/initializers/rack_attack.rb
+++ b/config/initializers/rack_attack.rb
@@ -46,10 +46,7 @@ class Rack::Attack
 
   PROTECTED_PATHS_REGEX = Regexp.union(PROTECTED_PATHS.map { |path| /\A#{Regexp.escape(path)}/ })
 
-  # Always allow requests from localhost
-  # (blocklist & throttles are skipped)
   Rack::Attack.safelist('allow from localhost') do |req|
-    # Requests are allowed if the return value is truthy
     req.remote_ip == '127.0.0.1' || req.remote_ip == '::1'
   end
 

diff --git a/spec/middleware/handle_bad_encoding_middleware_spec.rb b/spec/middleware/handle_bad_encoding_middleware_spec.rb
new file mode 100644 (file)
index 0000000..8c0d24f
--- /dev/null
+++ b/spec/middleware/handle_bad_encoding_middleware_spec.rb
@@ -0,0 +1,21 @@
+require 'rails_helper'
+
+RSpec.describe HandleBadEncodingMiddleware do
+  let(:app) { double() }
+  let(:middleware) { HandleBadEncodingMiddleware.new(app) }
+
+  it "request with query string is unchanged" do
+    expect(app).to receive(:call).with("PATH" => "/some/path", "QUERY_STRING" => "name=fred")
+    middleware.call("PATH" => "/some/path", "QUERY_STRING" => "name=fred")
+  end
+
+  it "request with no query string is unchanged" do
+    expect(app).to receive(:call).with("PATH" => "/some/path")
+    middleware.call("PATH" => "/some/path")
+  end
+
+  it "request with invalid encoding in query string drops query string" do
+    expect(app).to receive(:call).with("QUERY_STRING" => "", "PATH" => "/some/path")
+    middleware.call("QUERY_STRING" => "q=%2Fsearch%2Fall%Forder%3Ddescending%26page%3D5%26sort%3Dcreated_at", "PATH" => "/some/path")
+  end
+end