From dc1a830d3d59035ec38ef4d1ca3a9207c8c7a7a3 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 28 Jul 2016 11:42:06 -0400 Subject: [PATCH] SECURITY: SQL Injection in Admin List Active Users --- lib/admin_user_index_query.rb | 14 +++++++++++++- spec/components/admin_user_index_query_spec.rb | 10 ++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/admin_user_index_query.rb b/lib/admin_user_index_query.rb index 052e8f060..0cf304d42 100644 --- a/lib/admin_user_index_query.rb +++ b/lib/admin_user_index_query.rb @@ -18,8 +18,20 @@ class AdminUserIndexQuery find_users_query.count end + def self.orderable_columns + %w(created_at days_visited posts_read_count topics_entered post_count trust_level) + end + def initialize_query_with_order(klass) - order = [params[:order]] + order = [] + + custom_order = params[:order] + if custom_order.present? && + without_dir = custom_order.downcase.sub(/ (asc|desc)$/, '') + if AdminUserIndexQuery.orderable_columns.include?(without_dir) + order << custom_order + end + end if params[:query] == "active" order << "COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC" diff --git a/spec/components/admin_user_index_query_spec.rb b/spec/components/admin_user_index_query_spec.rb index 071120bd9..7d03cf1e3 100644 --- a/spec/components/admin_user_index_query_spec.rb +++ b/spec/components/admin_user_index_query_spec.rb @@ -16,6 +16,16 @@ describe AdminUserIndexQuery do query = ::AdminUserIndexQuery.new({ query: "active" }) expect(query.find_users_query.to_sql).to match("last_seen_at") end + + it "can't be injected" do + query = ::AdminUserIndexQuery.new({ order: "wat, no" }) + expect(query.find_users_query.to_sql).not_to match("wat, no") + end + + it "allows custom ordering" do + query = ::AdminUserIndexQuery.new({ order: "trust_level DESC" }) + expect(query.find_users_query.to_sql).to match("trust_level DESC") + end end describe "no users with trust level" do