1

Would the code below be considered "secure"?

Public Function GetManager(ByVal uname As String) As String
    Dim strSelect = String.Format("SELECT UserName FROM aspnet_Users INNER JOIN Hierarchy ON UserId = LineManagerID WHERE StaffID = (SELECT UserId FROM aspnet_Users WHERE UserName = '{0}')", uname)
    Dim cmdCommand = New SqlCommand(strSelect, _connection)
    GetManager = cmdCommand.ExecuteScalar()
End Function

I'm looking specifically at the part..

 where UserName = {'0'}, uname

Am I right in saying that the apostrophes open me up to some potential scripting attacks?

Many thanks.

DS

dstewart101
  • 1,084
  • 1
  • 16
  • 38
  • 3
    Use parameters to prevent sql injection. – Edper Jan 31 '15 at 10:59
  • hi - thanks for replying so quickly - could you offer a modified example to illustrate your point here, please? – dstewart101 Jan 31 '15 at 11:02
  • 1
    What control do you have on the parameter passed to this function? Could someone write `uname=';DROP TABLE ASPNET_USERS;--` – Steve Jan 31 '15 at 11:03
  • 1
    See this link : http://stackoverflow.com/questions/542510/how-do-i-create-a-parameterized-sql-query-why-should-i – Edper Jan 31 '15 at 11:06

1 Answers1

2

No, it's not secure. Your uname-parameter will be inserted into the string that is the SQL command. An evil user could send any string that messes with your sql.

The best thing is to use parameterized queries: How do I create a parameterized SQL query? Why Should I?

Community
  • 1
  • 1
idstam
  • 2,848
  • 1
  • 21
  • 30