From 752a2cc6548f0d9a02612f45450ec7f3f4a29713 Mon Sep 17 00:00:00 2001
From: Ted Johansson <ted@discourse.org>
Date: Tue, 5 Sep 2023 16:35:46 +0800
Subject: [PATCH] DEV: Handle bad parameters in TopicsController#wordpress
 (#23404)

We're seeing a large number of log noise from this endpoint due to malicious scanners that are trying to send clever params and seeing if they can break something.

This change simply rescues any NoMethodError during parameter parsing and re-raises a Discourse::InvalidParameters exception, which will be caught and render a 400.
---
 app/controllers/topics_controller.rb    | 26 ++++++++++++++-----------
 spec/requests/topics_controller_spec.rb |  6 ++++++
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 9b9016739c8..06eb231ff46 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -53,8 +53,8 @@ class TopicsController < ApplicationController
   def show
     flash["referer"] ||= request.referer[0..255] if request.referer
 
-    # We'd like to migrate the wordpress feed to another url. This keeps up backwards compatibility with
-    # existing installs.
+    # TODO: We'd like to migrate the wordpress feed to another url. This keeps up backwards
+    # compatibility with existing installs.
     return wordpress if params[:best].present?
 
     # work around people somehow sending in arrays,
@@ -212,15 +212,19 @@ class TopicsController < ApplicationController
       :only_moderator_liked,
     )
 
-    opts = {
-      best: params[:best].to_i,
-      min_trust_level: params[:min_trust_level] ? params[:min_trust_level].to_i : 1,
-      min_score: params[:min_score].to_i,
-      min_replies: params[:min_replies].to_i,
-      bypass_trust_level_score: params[:bypass_trust_level_score].to_i, # safe cause 0 means ignore
-      only_moderator_liked: params[:only_moderator_liked].to_s == "true",
-      exclude_hidden: true,
-    }
+    begin
+      opts = {
+        best: params[:best].to_i,
+        min_trust_level: params[:min_trust_level] ? params[:min_trust_level].to_i : 1,
+        min_score: params[:min_score].to_i,
+        min_replies: params[:min_replies].to_i,
+        bypass_trust_level_score: params[:bypass_trust_level_score].to_i, # safe cause 0 means ignore
+        only_moderator_liked: params[:only_moderator_liked].to_s == "true",
+        exclude_hidden: true,
+      }
+    rescue NoMethodError
+      raise Discourse::InvalidParameters
+    end
 
     @topic_view = TopicView.new(params[:topic_id], current_user, opts)
     discourse_expires_in 1.minute
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index 065d1dffab9..b772bf00d63 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -78,6 +78,12 @@ RSpec.describe TopicsController do
         "#{Discourse.base_url_no_prefix}#{moderator.avatar_template}",
       )
     end
+
+    it "does not error out when using invalid parameters" do
+      get "/t/#{p1.topic.id}/wordpress.json", params: { topic_id: 1, best: { leet: "haxx0r" } }
+
+      expect(response.status).to eq(400)
+    end
   end
 
   describe "#move_posts" do