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

Question

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?

Answer 1

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.

Answer 2

You might also use Pick.sanitize(str) to interpolate arbitrary text.

So, ignoring for a moment all the other ways to improve the query that mu mentioned, it would be:

Pick.find_by_sql("WITH cte1 AS (SELECT DISTINCT user_id, state, pick FROM picks 
WHERE id IN (#{Pick.sanitize pick_ids})), cte2 AS (SELECT user_id, coalesce(sum(amount_won), 0)
as picks_total_won FROM picks WHERE id IN (#{Pick.sanitize 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")