none
String sql and sql injections

    Question

  • Hey iam a lette confused. I have written at stored procedure like a STREN.G..My freind sad it is not a good way, it can by bee sql injection....this this right ?

     

    (But how can a convert this streng proc too ordanany sql then...?)

     

    Declare @sql nvarchar(4000)

    Declare @input varchar(20)

    Declare @xx varchar(20)

     

     

    Set @sql='Select * from thistabel'

     

    If @input<>' '

    Set @sql = @sql+'and t between 12 and 23'

    If @input<>425

    Set @sql = @sql+'and t = 23'

    if @xx=' '

    Set @sql = @sql+'order by name

    Saturday, October 18, 2008 4:52 PM

Answers

  • If you are not concatenating @input and / or @xx to @sql, then I do not see how we could inject t-sql code.

     

     

    AMB

    Saturday, October 18, 2008 5:03 PM
    Moderator
  • Technically, dynamic SQL can perform just as well as any other SQL for dynamic searches - it just has to be written properly.  The same goes for SQL Injection - improperly written dynamic SQL is definitely susceptable to SQL Injection, but if you write it properly, it is safe.  Did you read the articles I posted?  In your case, the one on Dynamic SQL is likely the best choice.  The author points out that the dynamic SQL solution is actually slightly more performant than the non-dynamic-SQL one.

    Does this help?
    Sunday, October 19, 2008 3:12 PM
    Moderator

All replies

  • If you are not concatenating @input and / or @xx to @sql, then I do not see how we could inject t-sql code.

     

     

    AMB

    Saturday, October 18, 2008 5:03 PM
    Moderator
  • I'm not sure if you've simplified your code, but the proc you've listed isn't at all susceptible to SQL Injection.  You're not generating dynamic sql by directly concatenating the input variables, you're using them as tests and hardcoding the SQL Concatenation - there's no risk there.  A few comments though:

    • If @input is a 20 character varchar, you should parenthesize the 425 to avoid implicit conversion errors: if @input <> '425'
    • You need spaces at the end of those strings (ex. 'and t between 12 and 23 '), and a closing single-quote at the end of your last line.
    SQL Injection is a very real concern, especially with dynamic SQL. Jonathan Kehayias has written two excellent articles on dynamic searches, with and without dynamic SQL, on the TSQL Wiki:

    http://code.msdn.microsoft.com/SQLExamples/Wiki/View.aspx?title=NonDynamicSearch&referringTitle=Home

    and

    http://code.msdn.microsoft.com/SQLExamples/Wiki/View.aspx?title=DynamicSearch&referringTitle=Home

    Hope this helps.
    Saturday, October 18, 2008 5:08 PM
    Moderator
  •  Yes i had simplified my code a lettet.

    I have in  my code, statment like this:

     

    SET @SQL = @SQL + ' AND Fir LIKE @Firs+''%'' ' + char(10)

     

    Do the String (dynamic) sql have bad performance then the ordanary(non dynamic) sql

     

    If i understand correct the dynamic sql can be effeicted by sql injection

    Sunday, October 19, 2008 7:03 AM
  • Technically, dynamic SQL can perform just as well as any other SQL for dynamic searches - it just has to be written properly.  The same goes for SQL Injection - improperly written dynamic SQL is definitely susceptable to SQL Injection, but if you write it properly, it is safe.  Did you read the articles I posted?  In your case, the one on Dynamic SQL is likely the best choice.  The author points out that the dynamic SQL solution is actually slightly more performant than the non-dynamic-SQL one.

    Does this help?
    Sunday, October 19, 2008 3:12 PM
    Moderator