CODEWITHSUNDEEP
ruby-on-railsrubysql-injection
Victor Policastro
Victor Policastro

Reputation: 59

Ruby on Rails SQL Injection - Building a query

I'm resolving all the SQL Injections in a system and I've found something that I don't know how to treat.

Can somebody help me?

Here is my method

def get_structure()
   #build query
   sql = %(
           SELECT pc.id AS "product_id", pc.code AS "code", pc.description AS "description", pc.family AS "family", 
                  p.code AS "father_code", p.description AS "father_description", 
                  p.family AS "father_family"
           FROM products pc
           LEFT JOIN imported_structures imp ON pc.id = imp.product_id
           LEFT JOIN products p ON imp.product_father_id = p.id
           WHERE pc.enable = true AND p.enable = true
   )
   #verify if there is any filter
   if !params[:code].blank?
     sql = sql + " AND UPPER(pc.code) LIKE '%#{params[:code].upcase}%'"
   end
   #many other parameters like the one above
   #execute query
   str = ProductStructure.find_by_sql(sql)
end

Thank you!

Upvotes: 2

Views: 870

Answers (2)

tadman
tadman

Reputation: 211560

You need to turn that into a placeholder value (?) and add the data as a separate argument. find_by_sql can take an array:

def get_structure
   #build query
   sql = %(SELECT...)
   query = [ sql ]

   if !params[:code].blank?
     sql << " AND UPPER(pc.code) LIKE ?"
     query << "%#{params[:code].upcase}%"
   end

   str = ProductStructure.find_by_sql(query)
end

Note, use << on String in preference to += when you can as it avoids making a copy.

Upvotes: 1

engineersmnky
engineersmnky

Reputation: 29318

You could use Arel which will escape for you, and is the underlying query builder for ActiveRecord/Rails. eg. 

products = Arel::Table.new("products")
products2 = Arel::Table.new("products", as: 'p')
imported_structs = Arel::Table.new("imported_structures")
query = products.project(
  products[:id].as('product_id'),
  products[:code],
  products[:description], 
  products[:family], 
  products2[:code].as('father_code'),
  products2[:description].as('father_description'),
  products2[:family].as('father_family')).
  join(imported_structs,Arel::Nodes::OuterJoin).
    on(imported_structs[:product_id].eq(products[:id])).
  join(products2,Arel::Nodes::OuterJoin).
    on(products2[:id].eq(imported_structs[:product_father_id])).
  where(products[:enable].eq(true).and(products2[:enable].eq(true)))
if !params[:code].blank?
  query.where(
     Arel::Nodes::NamedFunction.new('UPPER',[products[:code]])
       .matches("%#{params[:code].to_s.upcase}%")
  )
end

SQL result: (with params[:code] = "' OR 1=1 --test")

SELECT 
  [products].[id] AS product_id, 
  [products].[code], 
  [products].[description], 
  [products].[family], 
  [p].[code] AS father_code, 
  [p].[description] AS father_description, 
  [p].[family] AS father_family 
FROM 
  [products] 
  LEFT OUTER JOIN [imported_structures] ON [imported_structures].[product_id] = [products].[id] 
  LEFT OUTER JOIN [products] [p] ON [p].[id] = [imported_structures].[product_father_id] 
WHERE 
  [products].[enable] = true AND 
  [p].[enable] = true  AND 
  UPPER([products].[code]) LIKE N'%'' OR 1=1 --test%'

To use 

ProductStructure.find_by_sql(query.to_sql)

I prefer Arel, when available, over String queries because:

  • it supports escaping
  • it leverages your existing connection adapter for sytnax (so it is portable if you change databases)
  • it is built in code so statement order does not matter
  • it is far more dynamic and maintainable
  • it is natively supported by ActiveRecord
  • you can build any complex query you can possibly imagine (including complex joins, CTEs, etc.)
  • it is still very readable

Upvotes: 4

Related Questions