December 16, 2013 at 5:16 am
Hello.
I have a bad feeling about sp_executesql, I think in some particular situations it modify itself.
I mean if we have a select stored procedure for dynamic table name like this:
CREATE PROCEDURE SP @TableName AS SYSNAME AS
BEGIN
DECLARE @sql AS NVARCHAR(MAX)
SET @sql = N'Select something from ' + QUOTENAME(@TableName) + ' where 1=1'
EXEC sp_executesql @sql
END
I think after we pass something to @TableName parameter, in a particular situation its possible our stored procedure modify itself to something like this:
Select something from tableName where 1=1
And next time we want to pass something to @TableName, it will throw an exception that @TableName parameter not found.
I found something like this in my program and im not sure if my theory about sp_executesql modify itself randomly is correct or not.
Thank you for help.
___________________________________
Computer Enterprise Masoud Keshavarz
I don't care about hell.
If I go there I've played enough Diablo to know how to fight my way out.
December 16, 2013 at 5:29 am
A stored procedure cannot be changed without the ALTER PROCEDURE statement. Your statement the stored procedure "has changed itself" indicates someone executed code to drop/recreate or alter the existing stored procedure.You can look in SSMS at the properties of a stored procedure to see the date the procedure was created. This could help you when the modification has been done with a DROP/CREATE statement.
If this happens without your knowledge and you want logging when it happens consider creating a DDL trigger on the database.
December 16, 2013 at 5:39 am
Much appreciated, I think you've gave me enough clue.
___________________________________
Computer Enterprise Masoud Keshavarz
I don't care about hell.
If I go there I've played enough Diablo to know how to fight my way out.
December 16, 2013 at 6:33 am
As you have it currently configured, you're right not to trust it. You're completely open to SQL Injection. For a good laugh, and a good education, see to Bobby Tables[/url]. As you're using it, you're begging for trouble. sp_executesql does support parameters, but, you won't be able to just pass it table names like that. If you really want to do that (and I'm convinced it's a poor choice), you need to first do some type of check against the parameter values to ensure that what is passed is actually a real table name. Check in sys.objects or something. Just don't leave it raw like that.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
December 16, 2013 at 7:23 am
Grant is right on. Your use of sp_executeSQL is superfluous in terms of preventing injection the way you have implemented it. Read BOL on sp_executeSQL and learn how it works. It is always important to do as much parameter validation at the application level as possible even when using sp_executesql. Parameter validation inside stored procedures is also valid.
The probability of survival is inversely proportional to the angle of arrival.
December 24, 2013 at 3:10 am
Much appreciated, Ive read from somewhere if I use QUOTENAME(@FieldName,'''') with 4 single quotations, it makes stored procedure bullet proof from sql injection, but Im still in doubt if its adequate.
According to documentations QUOTENAME('Syntax-Example','''') produce 'Syntax-Example' out put.
But documentation didnt described how QUOTENAME('Syntax-Example','''') produce 'Syntax-Example' out put.
Is there any logic behind this? Or its just a rule?
___________________________________
Computer Enterprise Masoud Keshavarz
I don't care about hell.
If I go there I've played enough Diablo to know how to fight my way out.
December 24, 2013 at 3:21 am
masoudk1990 (12/24/2013)
Much appreciated, Ive read from somewhere if I use QUOTENAME(@FieldName,'''') with 4 single quotations, it makes stored procedure bullet proof from sql injection, but Im still in doubt if its adequate.According to documentations QUOTENAME('Syntax-Example','''') produce 'Syntax-Example' out put.
But documentation didnt described how QUOTENAME('Syntax-Example','''') produce 'Syntax-Example' out put.
Is there any logic behind this? Or its just a rule?
with regards to sql injection there is no such thing a bullet proof dynamic sql, unless you really build testing logic to intercept potential abuse !
Anyone anticipation your quotename can just anticipate your quote-char "* ; <put malicious code here> --" and you're *** [clipped on purpose] 😎
http://technet.microsoft.com/en-us/library/ms176114.aspx
Johan
Learn to play, play to learn !
Dont drive faster than your guardian angel can fly ...
but keeping both feet on the ground wont get you anywhere :w00t:
- How to post Performance Problems
- How to post data/code to get the best help[/url]
- How to prevent a sore throat after hours of presenting ppt
press F1 for solution, press shift+F1 for urgent solution 😀
Need a bit of Powershell? How about this
Who am I ? Sometimes this is me but most of the time this is me
December 24, 2013 at 3:52 am
masoudk1990 (12/24/2013)
Much appreciated, Ive read from somewhere if I use QUOTENAME(@FieldName,'''') with 4 single quotations, it makes stored procedure bullet proof from sql injection, but Im still in doubt if its adequate.According to documentations QUOTENAME('Syntax-Example','''') produce 'Syntax-Example' out put.
But documentation didnt described how QUOTENAME('Syntax-Example','''') produce 'Syntax-Example' out put.
Is there any logic behind this? Or its just a rule?
There are several things you can do to prevent SQL Injection. First, don't use dynamic T-SQL at all. Only use straight T-SQL and parameters. That code can't be modified by values within the parameters. Second, if you must use dynamic T-SQL, only use parameters as parameters in the same way you would with standard T-SQL. There are examples here in the Books Online. Finally, as has been said, put in logic checks to completely sanitize your inputs if you have to dynamically build T-SQL, especially if you're going to go down the dangerous route of passing in table names as values.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
December 24, 2013 at 6:08 am
Thanks Grant Fritchey, Ill take a look at that.
According to this:
http://technet.microsoft.com/en-us/library/ms176114.aspx
Now to add to my troubles:
how QUOTENAME('abc[]def') produce [abc[]]def] ? 😀
Well, Its a new question. Ill ask it in a new thread, thank you every one.
___________________________________
Computer Enterprise Masoud Keshavarz
I don't care about hell.
If I go there I've played enough Diablo to know how to fight my way out.
December 24, 2013 at 11:33 am
Ah... while I agree with being paranoid about the use of dynamic SQL, it should not be something that keeps you from using it when necessary. It's an incredibly powerful tool that can do things not otherwise possible but, as the others have stated, must be made to completely and absolutely prevent SQL Injection attempts.
For the code given in the original post, the content of @TableName should be verified by checking sys.objects to see if the table name actually exists and THAT can certainly be done without dynamic SQL. If the table name doesn't exist, the exit the stored procedure without giving any clue as to what the problem is so that a potential attacker isn't given any clues.
--Jeff Moden
Change is inevitable... Change for the better is not.
December 25, 2013 at 9:33 am
In my opinion there is no need to checking sys.objects for the code given in the original post 😀
DECLARE @sql AS NVARCHAR(MAX)
SET @sql = N'Select something from ' + QUOTENAME(@TableName) + ' where 1=1'
EXEC sp_executesql @sql
Guys! In practice no one ask table names as input from final users in their applications 🙂
All they do is using it hard coded.
But as I reading from here
http://blogs.msdn.com/b/raulga/archive/2007/01/04/dynamic-sql-sql-injection.aspx
You can easily prevent sql injection by using Parameterization
CREATE PROC [sp_select]( @name NVARCHAR(50) )
AS
declare @cmd nvarchar(max)
set @cmd = N'SELECT * FROM [users] WHERE NAME = @name'
EXEC sp_executesql @cmd, @name = @name
go
Now if attacker write:
set @name = 'Some Name''; GRANT CONTROL TO [Malicious User];--
Parameterization will take care of single quotations for us :-
set @cmd = N'SELECT * FROM [users] WHERE NAME = ''''Some Name''; GRANT CONTROL TO [Malicious User];--''''
It will take a while until I test it.
___________________________________
Computer Enterprise Masoud Keshavarz
I don't care about hell.
If I go there I've played enough Diablo to know how to fight my way out.
December 25, 2013 at 10:05 am
Using sp_executesql with parameters does NOT do a replace like this:
@cmd = REPLACE(@cmd, N'@name', @name);
It generates inline code like this:
set @name = 'Some Name''; GRANT CONTROL TO [Malicious User];--'
SELECT * from [users] WHERE NAME = @name;
sp_executesql used with parameters and no concatenation/REPLACE completely removes the ability to rewrite the SQL inside. Whatever garbage the user provides is just a string, a string that is a value that is evaluated as data, not as code. As long as you never use user provided data as part of the composition of the batch to be executed, sq_executesql will prevent SQL injection.
Sincerely,
Daniel
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply