SECURITY: Validate period param for top topic routes (#13818)

Fixes a possible SQL injection vector

lib/topic_query.rb

@@ -266,6 +266,10 @@ class TopicQuery
   end
 
   def list_top_for(period)
+    if !TopTopic.periods.include?(period.to_sym)
+      raise Discourse::InvalidParameters.new("Invalid period. Valid periods are #{TopTopic.periods.join(", ")}")
+    end
+
     score = "#{period}_score"
     create_list(:top, unordered: true) do |topics|
       topics = remove_muted_categories(topics, @user)

spec/components/topic_query_spec.rb

@@ -355,6 +355,24 @@ describe TopicQuery do
     end
   end
 
+  context "#list_top_for" do
+    it "lists top for the week" do
+      Fabricate(:topic, like_count: 1000, posts_count: 100)
+      TopTopic.refresh!
+      expect(topic_query.list_top_for(:weekly).topics.count).to eq(1)
+    end
+
+    it "only allows periods defined by TopTopic.periods" do
+      expect { topic_query.list_top_for(:all) }.not_to raise_error
+      expect { topic_query.list_top_for(:yearly) }.not_to raise_error
+      expect { topic_query.list_top_for(:quarterly) }.not_to raise_error
+      expect { topic_query.list_top_for(:monthly) }.not_to raise_error
+      expect { topic_query.list_top_for(:weekly) }.not_to raise_error
+      expect { topic_query.list_top_for(:daily) }.not_to raise_error
+      expect { topic_query.list_top_for("some bad input") }.to raise_error(Discourse::InvalidParameters)
+    end
+  end
+
   context 'mute_all_categories_by_default' do
     fab!(:category) { Fabricate(:category_with_definition) }
     fab!(:topic) { Fabricate(:topic, category: category) }

spec/requests/list_controller_spec.rb

@@ -410,6 +410,11 @@ RSpec.describe ListController do
       expect(response.media_type).to eq('application/rss+xml')
     end
 
+    it 'errors for invalid periods on top RSS' do
+      get "/top.rss?period=decadely"
+      expect(response.status).to eq(400)
+    end
+
     TopTopic.periods.each do |period|
       it "renders #{period} top RSS" do
         get "/top.rss?period=#{period}"