Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Is the following sql open to a sql injection attack?

...code extract:

  @yourSearch_1         varchar(20) 
, @yourSearch_2         varchar(20) 
, @yourSearch_3         varchar(20) 
, @yourSearch_4         varchar(20) 


SELECT  COUNT(*) AS Total
FROM        myTable
WHERE   (   (col1 LIKE @yourSearch_1 + '%' AND @Search_1 = 1)   OR  @Search_1 = 0 )
    AND (   (col2 LIKE @yourSearch_2 + '%' AND @Search_2 = 1)   OR  @Search_2 = 0 )
    AND (   (col3 LIKE @yourSearch_3 + '%' AND @Search_3 = 1)   OR  @Search_3 = 0 )
    AND (   (col4 LIKE @yourSearch_4 + '%' AND @Search_4 = 1)   OR  @Search_4 = 0 )
share|improve this question
Not every DBMS would accept the "@variable + '%'" notation - in fact, the '@' notation is rather DBMS-specific too. However, since the SQL uses placeholders rather than building the SQL from a string using variables, it is safe from SQL injection. – Jonathan Leffler Feb 6 '11 at 17:57

1 Answer

up vote 7 down vote accepted

Sql injection is a matter for where the sql is generated. What you have shown is safe, but if its generated dynamically, then its the code that does the generation which would need to be examined.

share|improve this answer
That isn't completely true and his code is still vulnerable, I sadly do not have a practical attack against it...yet – Woot4Moo Feb 6 '11 at 3:22
4  
@Woot4Moo, really? I'm curious to know how sql injection could be achieved here. – Brian Reichle Feb 6 '11 at 6:02
@Brian it does become a little more difficult to demonstrate because of the 20 char limit. – Woot4Moo Feb 6 '11 at 15:48
@Woot4Moo: pretend that they're VARCHAR(255) variables - how would you get an SQL Injection Attack to work against the code. (And if 255 is too short, increase the size until it fits your exploit - up to say 8 KiB.) – Jonathan Leffler Feb 6 '11 at 17:58
@Jonathan I will start testing against a sql server database at this moment and let you know anything I find, if you aren't using sql server let me know and I can make it compliant as need be – Woot4Moo Feb 6 '11 at 18:02
show 6 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.