From f33433bf9e0c1b57c5862d8e5c7ae28579b5bd3b Mon Sep 17 00:00:00 2001
From: Gerhard Schlager <mail@gerhard-schlager.at>
Date: Mon, 3 Sep 2018 06:45:32 +0200
Subject: [PATCH] Validation of params should restrict to max int (#6331)

* FIX: Validation of params should restrict to max int

* FIX: Send status 400 when "page" param isn't between 1 and max int
---
 app/controllers/list_controller.rb    |  1 -
 lib/topic_query.rb                    | 14 ++++++++++----
 spec/requests/list_controller_spec.rb | 27 +++++++++++++++++++--------
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb
index 1661a1fe5eb..a26aa6eff15 100644
--- a/app/controllers/list_controller.rb
+++ b/app/controllers/list_controller.rb
@@ -367,7 +367,6 @@ class ListController < ApplicationController
 
   def build_topic_list_options
     options = {}
-    params[:page] = params[:page].to_i rescue 1
     params[:tags] = [params[:tag_id].parameterize] if params[:tag_id].present? && guardian.can_tag_pms?
 
     TopicQuery.public_valid_options.each do |key|
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index c01d7c8567e..f5cc2b4597b 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -9,6 +9,7 @@ require_dependency 'topic_query_sql'
 require_dependency 'avatar_lookup'
 
 class TopicQuery
+  PG_MAX_INT ||= 2147483647
 
   def self.validators
     @validators ||= begin
@@ -17,8 +18,12 @@ class TopicQuery
         Integer === x || (String === x && x.match?(/^-?[0-9]+$/))
       end
 
-      zero_or_more = lambda do |x|
-        int.call(x) && x.to_i >= 0
+      zero_up_to_max_int = lambda do |x|
+        int.call(x) && x.to_i.between?(0, PG_MAX_INT)
+      end
+
+      one_up_to_max_int = lambda do |x|
+        int.call(x) && x.to_i.between?(1, PG_MAX_INT)
       end
 
       array_int_or_int = lambda do |x|
@@ -28,8 +33,9 @@ class TopicQuery
       end
 
       {
-        max_posts: zero_or_more,
-        min_posts: zero_or_more,
+        max_posts: zero_up_to_max_int,
+        min_posts: zero_up_to_max_int,
+        page: one_up_to_max_int,
         exclude_category_ids: array_int_or_int
       }
     end
diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb
index 7791f2dda32..86d276af7ce 100644
--- a/spec/requests/list_controller_spec.rb
+++ b/spec/requests/list_controller_spec.rb
@@ -11,11 +11,6 @@ RSpec.describe ListController do
   end
 
   describe '#index' do
-    it "doesn't throw an error with a negative page" do
-      get "/#{Discourse.anonymous_filters[1]}", params: { page: -1024 }
-      expect(response.status).to eq(200)
-    end
-
     it "does not return a 500 for invalid input" do
       get "/latest?min_posts=bob"
       expect(response.status).to eq(400)
@@ -28,6 +23,21 @@ RSpec.describe ListController do
 
       get "/latest?exclude_category_ids[]=bob"
       expect(response.status).to eq(400)
+
+      get "/latest?max_posts=1111111111111111111111111111111111111111"
+      expect(response.status).to eq(400)
+
+      get "/latest?page=-1"
+      expect(response.status).to eq(400)
+
+      get "/latest?page=0"
+      expect(response.status).to eq(400)
+
+      get "/latest?page=2147483648"
+      expect(response.status).to eq(400)
+
+      get "/latest?page=1111111111111111111111111111111111111111"
+      expect(response.status).to eq(400)
     end
 
     it "returns 200 for legit requests" do
@@ -42,10 +52,11 @@ RSpec.describe ListController do
 
       get "/latest.json?min_posts=0"
       expect(response.status).to eq(200)
-    end
 
-    it "doesn't throw an error with page params as an array" do
-      get "/#{Discourse.anonymous_filters[1]}", params: { page: ['7'] }
+      get "/latest?page=1"
+      expect(response.status).to eq(200)
+
+      get "/latest.json?page=2147483647"
       expect(response.status).to eq(200)
     end