Search code examples
sqlruby-on-railspostgresqlsql-injectionrails-activerecord

Rails - how to protect against sql injection for a postgres find_by_sql query?


I have the following find_by_sql query against a postgres DB:

pick_ids = picks.pluck(:id).join(',')

Pick.find_by_sql("WITH cte1 AS (SELECT DISTINCT user_id, state, pick FROM picks 
WHERE id IN (#{pick_ids})), cte2 AS (SELECT user_id, coalesce(sum(amount_won), 0)
as picks_total_won FROM picks WHERE id IN (#{pick_ids}) GROUP BY user_id), cte3 AS 
(SELECT user_id, COUNT(CASE WHEN state = 'won' then 1 ELSE null END) AS picks_won,
FROM cte1 GROUP BY user_id) SELECT cte2.picks_total_won, cte3.picks_won,
FROM cte2 INNER JOIN cte3 ON cte2.user_id = cte3.user_id")

When I try to parameterize it (i.e., ?, ?...pick_ids, pick_ids), however, I get the following error:

ArgumentError: wrong number of arguments (3 for 1..2)

(1) Can you parameterize a find_by_sql query like this? If so, how?

(2) Do you even need to worry about SQL injection if the query will never receive user-inputted params?


Solution

  • First of all, you might want to use a %Q{...} string and some formatting in your SQL to avoid an unreadable mess:

    Pick.find_by_sql(%Q{
      WITH
        cte1 AS (
          SELECT DISTINCT user_id, state, pick
          FROM picks 
          WHERE id IN (#{pick_ids})
        ),
        cte2 AS (
          SELECT user_id, coalesce(sum(amount_won), 0) as picks_total_won
          FROM picks
          WHERE id IN (#{pick_ids})
          GROUP BY user_id
        ),
        cte3 AS (
          SELECT user_id, COUNT(CASE WHEN state = 'won' then 1 ELSE null END) AS picks_won,
          FROM cte1
          GROUP BY user_id
        )
      SELECT cte2.picks_total_won, cte3.picks_won
      FROM cte2
      JOIN cte3 ON cte2.user_id = cte3.user_id
    })
    

    Depending on where your picks come from, you might be able to avoid the interpolation completely and embed the SQL to generate the pick_ids (possibly using another CTE).

    If you need to feed the pick_ids in from the outside world then you can use placeholders with find_by_sql but the interface is a bit bizarre: you have to pass an array to find_by_sql:

    Pick.find_by_sql([%Q{
      WITH
        cte1 AS (
          SELECT DISTINCT user_id, state, pick
          FROM picks 
          WHERE id IN (:pick_ids)
        ),
        cte2 AS (
          SELECT user_id, coalesce(sum(amount_won), 0) as picks_total_won
          FROM picks
          WHERE id IN (:pick_ids)
          GROUP BY user_id
        ),
        cte3 AS (
          SELECT user_id, COUNT(CASE WHEN state = 'won' then 1 ELSE null END) AS picks_won,
          FROM cte1
          GROUP BY user_id
        )
      SELECT cte2.picks_total_won, cte3.picks_won
      FROM cte2
      JOIN cte3 ON cte2.user_id = cte3.user_id
    }, :pick_ids => some_array_of_ids])
    

    Note the location of the [ and ] in the argument list.