SQL Injection

[StageLeft]

I am looking at some VB DLLs that serve as a data layer for some old ASP that are calling SQL 2000 stored procedures. These procedures do not consist of dynamic SQL. They only take parameters and look like:

SELECT User FROM Users WHERE [name] = @Param1

Based on my understanding, this is invulnerable to an injection attack because, regardless of the contents of @Param1 (which could be anything, fed in from the web pages through to the VB DLL), SQL will still try to do matches of [name] to @Param1. If it was all dynamic, I know additional statements could piggy back.

My question, though is, how they're called from the VB. In some cases the parametes for the sprocs are specific, binding variables to a parameter and then executed and Bob's your uncle. However, in a few cases we have code that looks like this:

sql = "<stored procedure> " & 'Bob'
oConn.Execute sql

Are these calls susceptible? Does a dynamic appending of parameters in the above manner to a stored procedure that itself has a) no dynamic SQL and b) specific parameters within it mean this code is still protected from injection at the database layer even if the data layer itself would appear vulnerable?

Thanks!!!!!!

 

[torpid]

I assume they are vulnerable. Instead of Bob couldn't you put:

Bob'; some malicious sql ending with '

 

[tfinch2]

Regardless if the API is secure, you should be sanitizing your input.

 

[EagleKeeper]

Originally posted by: tfinch2
Regardless if the API is secure, you should be sanitizing your input.

If you do not filter your input, then the potential for trouble exists.

 

[StageLeft]

Originally posted by: torpid
I assume they are vulnerable. Instead of Bob couldn't you put:

Bob'; some malicious sql ending with '

This sounds reasonable. So even if I assume that the stored procedure itself is secured, the .Execute sql command would be able to insert two commands, so unless oConn.Execute is looking specifically only for a single command, which seems unlikely, it would basically execute as many as are fed into it, delimited in some way.

Regardless if the API is secure, you should be sanitizing your input.

That's going in place, too, but I really need to secure this as well for comfort's sake.

 

[JACKDRUID]

store procedure is secure. what is not secure is adhoc SQL statements.
you should be fine here. sql = "<stored procedure> " & 'Bob' because you are executing <store procedure>, not 'bob'

in SELECT User FROM Users WHERE [name] = @Param1 , you are executing the whole thing.

 

[StageLeft]

Originally posted by: JACKDRUID
store procedure is secure. what is not secure is adhoc SQL statements.
you should be fine here. sql = "<stored procedure> " & 'Bob' because you are executing <store procedure>, not 'bob'

in SELECT User FROM Users WHERE [name] = @Param1 , you are executing the whole thing.

What do you think of torpid's comment about the connection's execution method being able to execute multiple statements and thus, even if the stored procedure is locked down, other things can piggy back on top?

The way security is for this user (SQL account) is read-only on the entire DB. It's also an owner, unfortunately, but has deny write-access, so my intent is to give execute permission to all pre-cleared sprocs and routines that call them.

 

[Crusty]

Originally posted by: Skoorb

Originally posted by: JACKDRUID
store procedure is secure. what is not secure is adhoc SQL statements.
you should be fine here. sql = "<stored procedure> " & 'Bob' because you are executing <store procedure>, not 'bob'

in SELECT User FROM Users WHERE [name] = @Param1 , you are executing the whole thing.

What do you think of torpid's comment about the connection's execution method being able to execute multiple statements and thus, even if the stored procedure is locked down, other things can piggy back on top?

The way security is for this user (SQL account) is read-only on the entire DB. It's also an owner, unfortunately, but has deny write-access, so my intent is to give execute permission to all pre-cleared sprocs and routines that call them.

In that case it won't matter how secure the sproc is if the execute command allows for more then one SQL statement to be executed. It's better to assume it does and protect against it then to hope that it won't

 

[JACKDRUID]

Originally posted by: Skoorb

Originally posted by: JACKDRUID
store procedure is secure. what is not secure is adhoc SQL statements.
you should be fine here. sql = "<stored procedure> " & 'Bob' because you are executing <store procedure>, not 'bob'

in SELECT User FROM Users WHERE [name] = @Param1 , you are executing the whole thing.

What do you think of torpid's comment about the connection's execution method being able to execute multiple statements and thus, even if the stored procedure is locked down, other things can piggy back on top?

The way security is for this user (SQL account) is read-only on the entire DB. It's also an owner, unfortunately, but has deny write-access, so my intent is to give execute permission to all pre-cleared sprocs and routines that call them.

I stand corrected, but exec statement execute only the stored procedure, with its parameter appended into it as a variable.

so anything after the stored procedure will be seen as "anything", and will only be used to match not executed.

 

[oog]

isn't this a case of using prepared statements?

 

[StageLeft]

As a follow-up, I've done some fiddling.

If we're in SQL Server itself with a query window up:

EXEC <Procedure> @SomeVar

Will exectue only the procedure. If the @SomeVar has good data and also some naughty stuff, the procedure will work properly because @SomeVar is only a parameter; if it has extra statements, these don't appear to run as statements and they are indeed considered as the same parameter.

However, with something like (still in a query window in SQL):

set @sql = N'<procedure> ' + @SomeVar

exec(@sql)

in this case, if @SomeVar is a legit parameter AND has a semi-colon and then some junk, like "drop table Users", the drop table users will definitely execute, so in this case the code in my example with the connection.execute is definitely susceptible to an injection attack.

I believe this is what jackdruid was saying, too!

 

[JACKDRUID]

wow I didn't know exec() would execute everything.

well, what we usually do here is
dim sqlcnn as new sqlconnectiion(your connection string)
sqlcnn.open
dim cmd as new sqlcommand("yourstoredprocedure",sqlcnn)
cmd.commandtype=commandtype.storedprocedure
dim somevar as Sqlparameter
somevar = new sqlparameter("@SomeVar",nvarchar(50))
somevar.direction = parameterdirection.input
cmd.parameters.add(somevar)
cmd.ExecuteNonQuery()

hope this helps.

 

[torpid]

Originally posted by: Skoorb
As a follow-up, I've done some fiddling.

If we're in SQL Server itself with a query window up:

EXEC <Procedure> @SomeVar

Will exectue only the procedure. If the @SomeVar has good data and also some naughty stuff, the procedure will work properly because @SomeVar is only a parameter; if it has extra statements, these don't appear to run as statements and they are indeed considered as the same parameter.

However, with something like (still in a query window in SQL):

set @sql = N'<procedure> ' + @SomeVar

exec(@sql)

in this case, if @SomeVar is a legit parameter AND has a semi-colon and then some junk, like "drop table Users", the drop table users will definitely execute, so in this case the code in my example with the connection.execute is definitely susceptible to an injection attack.

I believe this is what jackdruid was saying, too!

Yep, but if instead of EXEC <Procedure> @SomeVar you did

"EXEC <Procedure>' " + SomeVar + " ' "

Where SomeVar is input by the user, then you'd need to be concerned. I thought that was the scenario you were describing. Parameterizing it reduces the risk quite a bit. In that case, you just have to make sure that SomeVar isn't used in an exec or similar statement.