SQL Injection Technique For Query String

Question

I've been trying to ensure my code isn't susceptible to the infamous SQL Injection Attack. The question involves the Query String, the legacy code I'm managing has instances of inline SQL which applies:

string query = @"SELECT * FROM [Order] 
   WHERE ([Id]=" + Request.QueryString[@"oid"] + ");";

Obviously that is bad, it will take the attack. My question is this enough?

command.Parameters.AddWithValue("@OrderId", Request.QueryString[@"oid"]);

So now the query has a parameter, which is being passed a value. I know it has some form of encoding. However, will that be enough as any malicious attacker could exploit that query string? So should I do Encode on the query string? That way it will encode it safely to avoid being exploited any further?

Some clarification on the matter would be helpful.

You should check out Can we stop using AddWithValue() already? and stop using .AddWithValue() - it can lead to unexpected and surprising results... — marc_s
– marc_s, Commented Sep 4, 2014 at 16:57

D Stanley · Accepted Answer · 2014-09-04 17:00:17Z

4

is this enough?

No - you also need to change your query to

string query = @"SELECT * FROM [Order] 
   WHERE ([Id]=@OrderId);";

I know it has some form of encoding

No, it uses the actual value, but it does not inject it into the SQL statement. It treats it as a literal string, so there's no way to include punctuation or malicious code that will get interpreted as SQL.

EDIT

I may have misunderstood - you may need to URL-decode the value if it included URL-encoded characters (%20, &, etc. ), but no encoding (or decoding) is necessary to prevent SQL injection.

edited Sep 4, 2014 at 17:00

answered Sep 4, 2014 at 16:49

D Stanley

153k12 gold badges189 silver badges257 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Greg Over a year ago

I know the query has to be changed to use that parameter, should of added that to the question, sorry. So Parameters.AddWithValue should be enough because of how it converts?

D Stanley Over a year ago

@Greg It doesn't convert. It will insert the string value as-is. You do not need to worry about encoding or escaping characters.

Collectives™ on Stack Overflow

SQL Injection Technique For Query String

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related