[#59391] avoid sql injection

opf · Nov 19, 2024 · b13c7d0 · b13c7d0
1 parent 7e83713
commit b13c7d0
Showing 1 changed file with 20 additions and 7 deletions.
diff --git a/modules/storages/lib/api/v3/file_links/work_packages_file_links_api.rb b/modules/storages/lib/api/v3/file_links/work_packages_file_links_api.rb
@@ -31,23 +31,36 @@
 class API::V3::FileLinks::WorkPackagesFileLinksAPI < API::OpenProjectAPI
   helpers do
     def sync_and_convert_relation(file_links)
+      return ::Storages::FileLink.none if file_links.empty?
+
       sync_result = ::Storages::FileLinkSyncService
                       .new(user: current_user)
                       .call(file_links)
                       .result
 
-      value_list = sync_result
-                     .map { |file_link| "(#{file_link.id},'#{file_link.origin_status}')" }
-                     .join(",")
+      create_new_relation(sync_result)
+    end
 
-      origin_status_attribute = <<-SQL.squish
-        LEFT JOIN (VALUES #{value_list}) AS origin_status (id,status) ON origin_status.id = file_links.id
-      SQL
+    def create_new_relation(sync_result)
+      values = sync_result.map { |file_link| [file_link.id, file_link.origin_status.to_s] }
+
+      sanitized_sql = ActiveRecord::Base.send(
+        :sanitize_sql_array,
+        [origin_status_join(sync_result.size), *values.flatten]
+      )
 
       ::Storages::FileLink.where(id: sync_result.map(&:id))
-                          .joins(origin_status_attribute)
+                          .joins(sanitized_sql)
                           .select("file_links.*, origin_status.status AS origin_status")
     end
+
+    def origin_status_join(value_count)
+      placeholders = Array.new(value_count).map { "(?,?)" }.join(",")
+
+      <<-SQL.squish
+        LEFT JOIN (VALUES #{placeholders}) AS origin_status (id,status) ON origin_status.id = file_links.id
+      SQL
+    end
   end
 
   resources :file_links do