0

I have a tool which points out all the sql injection issues and I found one as follows :

"SELECT GB.BTN,GUP.CUST_USERNAME,GUP.EMAIL from GBS_BTN GB,GBS_USER_BTN GUB,GBS_USER_PROFILE GUP WHERE GB.BTN=GUB.BTN AND GUB.CUST_UID=GUP.CUST_UID AND GB.ET_ID='" + strAccountID + "' ORDER BY CREATE_DATE DESC",oCin"

can some please tell me how to construct the above query to avoid sql injection?

user1567194
  • 179
  • 2
  • 3
  • 17

3 Answers3

9

Option 1: Use parameterized queries instead of contencating strings.

More info can be found here: http://msdn.microsoft.com/en-us/library/ff648339.aspx

Option 2: Use parameterized Stored Procedures

Option 3 would be to escape the strings by using Replace() but this should be a last resort. It's weak, and there are ways around it.

string sql = "Select * From someTable where SomeStringField = '" + myVariable.REplace("'", "''") + "'";
David
  • 72,686
  • 18
  • 132
  • 173
2
SELECT foo, bar, etc FROM Bobby WHERE this = that
AND GB.ET_ID = @accountID ORDER BY mySort

Then, in the command variable, add parameters. Like this:

myCommand.Parameters.AddWithValue("@accountID", strAccountID);

You have to understand that the vulnerability lies in the fact that if strAccountID comes from a control which is editable by the user, it might contain something like:

' drop table GBS_BTN --

Which would cause your program to run part of the query, then delete the table.

Edit: and using parameters as in the example causes anything the user has typed to be escaped, so you're safe from this kind of exploit. As others have suggested, you could also use stored procedures. You'd then be forced to use parameters like in the example. Stored procedures have other characteristics that might be helpful for you, but that's another discussion.

Geeky Guy
  • 9,229
  • 4
  • 42
  • 62
0

SQL Injection may happen if an user manages to populate strAccountID with specially crafted content, forcing the statement to be ignored and then running arbitrary code.

This post may have some relevant information for you. AviD's answer is quite precise, so let me quote the important parts:

  • Whitelist validation: type, length, format or accepted values
  • If you want to blacklist, go right ahead. Quote escaping is good, but within context of the other mitigations.
  • Use Command and Parameter objects, to preparse and validate
  • Call parameterized queries only.
  • Better yet, use Stored Procedures exclusively.
  • Avoid using dynamic SQL, and dont use string concatenation to build queries.
  • If using SPs, you can also limit permissions in the database to executing the needed SPs only, and not access tables directly.
  • you can also easily verify that the entire codebase only accesses the DB through SPs

While I don't have as much love as he does for Stored Procedures (it doesn't prevent faulty/bad code, and an EXEC statement inside a SP that mishandle single quotes will fail to protect against SQL Injection), it's an interesting read.

Community
  • 1
  • 1
OnoSendai
  • 3,960
  • 2
  • 22
  • 46
  • 1
    Constructing database operations that include user-supplied inputs is a very legitimate need. While "avoid dynamic SQL" is reasonable general advice, it ignores that dynamically generated SQL is reasonably safe when constructed as part of a parameterized, PreparedStatement. I find this list too be too limiting and narrow. – scottb May 08 '13 at 21:11