(In reply to comment #0) > The code in question constructs an SQL statement using string concatenation for > the $pushids variable. This approach is vulnerable to SQL injection if user > controlled data is ever present within $pushids. The var is generated out of sql ids, so it should be safe. As far as I know, there is no way to use bind parameters with arrays and sql "IN" statements.
In that case lets add strict input validation on the input parameters so that we can gracefully handle these values without resulting in a SQL exception. Where you able to determine why a different numeric value, such as "123" would also cause the exception?
In that case, the query above would have no results, so the concat will result in an empty string.
The goal is to use input validation checks to determine if the URL arguments are valid. If they aren't, then we abort the SQL statements all together. This will accomplish two things: 1. No worries of SQL injection since the bad data would never make it to the SQL statement. (And we are binding all other parameters) 2. No SQL error message returned to our users. This is best practice from a security perspective.