Is this susceptible to an injection attack

Question

Is the following c# snippet susceptible to a SQL Injection Attack?

string sql = @"
    declare @sql navarchar(200)
    set @sql = 'select * from customers where customerId = ' + convert(navarchar, @custId)
    exec sp_sqlexec @sql
"

oSQL.Parameters.Add("custId", CustomerId);
DataTable dt = oSQL.ExecuteDataTable(sql);

I understand this is a trivial sql statement but I'm more interested in the approach of using exec sp_sqlexec. The sql statement is more dynamic than the one stated but don't think it's important for my question.

If `CustomerId` is a string that a user can input then yes. If it's a number then no. Why are you using dynamic SQL anyways? You can just execute that SQL directly. — D Stanley, Dec 09 '14 at 22:41
While @DStanley is technically correct, the main thing here (to me, at least) is that this basic approach is wide open to SQL injection. There's oodles of info on SO about it. — Andrew, Dec 09 '14 at 22:43
Sorry, I mean to clarify in OP...I know this is a trivial statement but I was going for more concept of running sp_sqlexec with parameterized query. Of course the Sql statement is more dynamic than that one statement. — Rod, Dec 09 '14 at 22:45
It's possible. I will suggest better think of having a ORM to execute queries. As you have it right now, you will have to validate that custID param. — Camilo Aguilar, Dec 09 '14 at 22:46
`sp_sqlexec` isn't even a supported system procedure since before SQL 2005 — EkoostikMartin, Dec 09 '14 at 22:50
@EkoostikMartin, I'm not following the reason for your comment (apologies) — Rod, Dec 09 '14 at 22:52
also see [this](http://stackoverflow.com/questions/3587814/can-my-users-inject-my-dynamic-sql) — Beth, Dec 09 '14 at 22:55

score 3 · Accepted Answer · answered Dec 09 '14 at 22:55

A slightly more secure solution would be to make the dynaimc query parameterized as well:

(Note that you should be using sp_executesql as well:

string sql = @"
    declare @sql navarchar(200)
    set @sql = 'select * from customers where customerId = @customerId'
    exec sp_sqlexecuseSQL @sql, N'@customerId INT`, @customerId = @custId
"

oSQL.Parameters.Add("custId", CustomerId);
DataTable dt = oSQL.ExecuteDataTable(sql);

logixologist · Answer 2 · 2014-12-09T22:54:23.933

Updating answer based on comments from SO. Dynamic SQL (or really any SQL statement, this is a good rule to follow) in general usually is open to SQL Injection if their is a potential for user input. If all your parameters that make the SQL statment say come from another database or say a dropdown, etc then No it is not succeptible to SQL Injection.

The general rule to remember: Don't ever allow unvalidated data to get into a SQL statement. Everything should be validated and add them to the database as a parameter.

Is this susceptible to an injection attack

2 Answers2