From 6c564c9ce38a31df9f14e1988f4065c4854516d8 Mon Sep 17 00:00:00 2001
From: Lucas Di Pentima <lucas@curoverse.com>
Date: Thu, 11 Aug 2016 21:38:46 -0300
Subject: [PATCH 1/1] 9333: Attribute validation on "select" to avoid invalid
 SQL statements

---
 .../app/controllers/application_controller.rb | 15 ++++++++++
 .../test/integration/collections_api_test.rb  | 28 +++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 3a888184f8..89bda3cdee 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -277,6 +277,21 @@ class ApplicationController < ActionController::Base
         # Map attribute names in @select to real column names, resolve
         # those to fully-qualified SQL column names, and pass the
         # resulting string to the select method.
+        if @select.empty?
+          raise ArgumentError.new("Attribute selection list cannot be empty")
+        end
+        api_column_map = model_class.attributes_required_columns
+        invalid_attrs = []
+        @select.each do |s|
+          next if ["href", "kind", "etag"].include? s
+          if not s.is_a? String
+            raise ArgumentError.new("Attribute '#{s}' should be a string")
+          end
+          invalid_attrs.append(s) if not api_column_map.include? s
+        end
+        if not invalid_attrs.empty?
+          raise ArgumentError.new("Invalid attribute(s): '#{invalid_attrs.join(', ')}'")
+        end
         columns_list = model_class.columns_for_attributes(@select).
           map { |s| "#{ar_table_name}.#{ActiveRecord::Base.connection.quote_column_name s}" }
         @objects = @objects.select(columns_list.join(", "))
diff --git a/services/api/test/integration/collections_api_test.rb b/services/api/test/integration/collections_api_test.rb
index 4251047cea..e67f1b1a9b 100644
--- a/services/api/test/integration/collections_api_test.rb
+++ b/services/api/test/integration/collections_api_test.rb
@@ -57,6 +57,34 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
     assert_equal "arvados#collectionList", json_response['kind']
   end
 
+  test "get index with select= (valid attribute)" do
+    get "/arvados/v1/collections", {
+          :format => :json,
+          :select => ['portable_data_hash'].to_json
+        }, auth(:active)
+    assert_response :success
+    assert json_response['items'][0].keys.include?('portable_data_hash')
+    assert not(json_response['items'][0].keys.include?('uuid'))
+  end
+
+  test "get index with select= (invalid attribute) responds 422" do
+    get "/arvados/v1/collections", {
+          :format => :json,
+          :select => ['bogus'].to_json
+        }, auth(:active)
+    assert_response 422
+    assert_match /Invalid attribute.*bogus/, json_response['errors'].join(' ')
+  end
+
+  test "get index with select= (invalid attribute type) responds 422" do
+    get "/arvados/v1/collections", {
+          :format => :json,
+          :select => [['bogus']].to_json
+        }, auth(:active)
+    assert_response 422
+    assert_match /Attribute.*should be a string/, json_response['errors'].join(' ')
+  end
+
   test "controller 404 response is json" do
     get "/arvados/v1/thingsthatdonotexist", {:format => :xml}, auth(:active)
     assert_response 404
-- 
2.30.2