SQL Clone
SQLServerCentral is supported by Redgate
 
Log in  ::  Register  ::  Not logged in
 
 
 


Avoiding injection on stored procedure


Avoiding injection on stored procedure

Author
Message
el_wise
el_wise
Forum Newbie
Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)

Group: General Forum Members
Points: 6 Visits: 12
Hi,

I've got an SP that looks like this:

CREATE PROCEDURE USPDeleteAnswers
@Id varchar(50),
@QuestionId varchar(100)
AS

BEGIN

DECLARE @SQL nvarchar(255)
SET @SQL = 'DELETE FROM tblUserAnswer where Id = @Id$ and QuestionId in (@QuestionId$)'
EXEC sp_executesql
@SQL,
N'@Id$ varchar(50), @QuestionId$ varchar(100)',
@Id$ = @Id,
@QuestionId$ = @QuestionId

END
GO



Now my problem is that I need to send several comma separated numbers to @QuestionId, ie. "12, 13, 14" so I can delete them all this in one go. But this is throwing an error "Error converting data type varchar to real"

Any ideas on how I could do this please?

Thanks
davidandrews13
davidandrews13
Ten Centuries
Ten Centuries (1.1K reputation)Ten Centuries (1.1K reputation)Ten Centuries (1.1K reputation)Ten Centuries (1.1K reputation)Ten Centuries (1.1K reputation)Ten Centuries (1.1K reputation)Ten Centuries (1.1K reputation)Ten Centuries (1.1K reputation)

Group: General Forum Members
Points: 1074 Visits: 4542
el_wise (3/18/2009)
Hi,

I've got an SP that looks like this:

CREATE PROCEDURE USPDeleteAnswers
@Id varchar(50),
@QuestionId varchar(100)
AS

BEGIN

DECLARE @SQL nvarchar(255)
SET @SQL = 'DELETE FROM tblUserAnswer where Id = @Id$ and QuestionId in (@QuestionId$)'
EXEC sp_executesql
@SQL,
N'@Id$ varchar(50), @QuestionId$ varchar(100)',
@Id$ = @Id,
@QuestionId$ = @QuestionId

END
GO



Now my problem is that I need to send several comma separated numbers to @QuestionId, ie. "12, 13, 14" so I can delete them all this in one go. But this is throwing an error "Error converting data type varchar to real"

Any ideas on how I could do this please?

Thanks












this is what i use:
here is the function:



REATE FUNCTION [dbo].[fnConvertToTableInt] (@ItemList as varchar(5000))
RETURNS @ParsedList table(UID int, Item int)
AS
BEGIN

DECLARE @Item varchar(50)
DECLARE @Pos int
DECLARE @UID int

SET @UID = 0

SET @ItemList = LTRIM(RTRIM(@ItemList))+ ','
SET @Pos = CHARINDEX(',', @ItemList, 1)

IF REPLACE(@ItemList, ',', '') <> ''
BEGIN
WHILE @Pos > 0
BEGIN
SET @Item = LTRIM(RTRIM(LEFT(@ItemList, @Pos - 1)))
IF (Select isNumeric(@Item)) = 1
BEGIN
INSERT INTO @ParsedList (Item,UID)
VALUES (CAST(@Item AS int) ,@UID)
END
SET @ItemList = RIGHT(@ItemList, LEN(@ItemList) - @Pos)
SET @Pos = CHARINDEX(',', @ItemList, 1)
SET @UID = @UID + 1

END
END
RETURN

END


you then call the function passing in your IDs as a string

select * from fnconverttotableint('101,201,301')

this will return the following table

UID ITEM
0......101
1.......201
2.......301



in your case you would then do something like



SET @SQL = 'DELETE FROM tblUserAnswer where Id = @Id$ and QuestionId in (Select Item from fnconverttotableint(@QuestionId))'
bitbucket-25253
bitbucket-25253
SSCertifiable
SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)

Group: General Forum Members
Points: 7931 Visits: 25280
Post deleted as it has been shown to be open to an injection attack

If everything seems to be going well, you have obviously overlooked something.

Ron

Please help us, help you -before posting a question please read

Before posting a performance problem please read
el_wise
el_wise
Forum Newbie
Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)Forum Newbie (6 reputation)

Group: General Forum Members
Points: 6 Visits: 12
Thanks,

bitbucket, that is the way I had it coded before, but that seems to be an easy target for SQL injection?

what do you reckon?

Thanks again
Michael Valentine Jones
Michael Valentine Jones
SSCertifiable
SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)

Group: General Forum Members
Points: 5860 Visits: 11771
bitbucket (3/18/2009)
Might I suggest:
CREATE PROCEDURE USPDeleteAnswers
@Id varchar(50),
@QId varchar(100)
AS
DECLARE @SQL nvarchar(255)
SET @SQL = 'DELETE FROM tblUserAnswer where Id = ''' + @Id + ''' and QuestionId in (' + @QId + ' )'
PRINT @SQL -- delete after testing
EXEC sp_executesql @SQL



Test run as:
Dbo.USPDeleteAnswers 'abcdeid', '1,5,20'
Then alter the SP to delete the print statement
Note that each passed parameter is passed in character / varchar format, that is do not forget the ' ' surrounding each passed value.


Since the title of this thread is “Avoiding injection on stored procedure”, have you considered what would happen if they passed the following as parameters:
Execute USPDeleteAnswers @Id = 'abc', @QuestionId = ' 1,3 ) or 1 = 1 --  '


bitbucket-25253
bitbucket-25253
SSCertifiable
SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)

Group: General Forum Members
Points: 7931 Visits: 25280
Post deleted as it has been shown to be insufficient to thwart an injection attack.

If everything seems to be going well, you have obviously overlooked something.

Ron

Please help us, help you -before posting a question please read

Before posting a performance problem please read
Michael Valentine Jones
Michael Valentine Jones
SSCertifiable
SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)

Group: General Forum Members
Points: 5860 Visits: 11771
bitbucket (3/18/2009)

Michael Valentine Jones
Since the title of this thread is “Avoiding injection on stored procedure”, have you considered what would happen if they passed the following as parameters:

[Execute USPDeleteAnswers @Id = 'abc', @QuestionId = ' 1,3 ) or 1 = 1 --

dbo.USPDeleteAnswers 'abc', '1=1' OR dbo.USPDeleteAnswers 'abc', 1=1
Will produce:
Msg 102, Level 15, State 1, Line 1
Incorrect syntax near '='.
Trying: DELETE FROM tblUserAnswer where Id = '*' and QuestionId in (1,3 )
Gives: (0 row(s) affected)

To further strengthen the SP set the length value for @Id to its length as defined in the database table, and of course set it in the tables definition to a realistic length and not the default value when creating a table using SSMS.
Also set the length for the parameter @QuestionId to a realistic value, remembering that when the passed value is longer than the defined length of the parameter in the procedure it is truncated.


The stored proc call example I posted before would result in the following dynamic SQL being executed, and would delete all the rows in the table.
DELETE FROM tblUserAnswer where Id = 'abc' and QuestionId in (1,3 ) or 1 = 1 --  )



el_wise has very good reason to be skeptical of that code. Much worse could be done, depending on the access level of the account running this command.
bitbucket-25253
bitbucket-25253
SSCertifiable
SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)SSCertifiable (7.9K reputation)

Group: General Forum Members
Points: 7931 Visits: 25280
Post deleted as it has been shown to be an avenue for an injection attack.

If everything seems to be going well, you have obviously overlooked something.

Ron

Please help us, help you -before posting a question please read

Before posting a performance problem please read
Sergiy
Sergiy
SSChampion
SSChampion (10K reputation)SSChampion (10K reputation)SSChampion (10K reputation)SSChampion (10K reputation)SSChampion (10K reputation)SSChampion (10K reputation)SSChampion (10K reputation)SSChampion (10K reputation)

Group: General Forum Members
Points: 10586 Visits: 11966
How about this:

[dbo].[USPDeleteAnswers] 'xxx','1,3,5) or 1=1--'
Michael Valentine Jones
Michael Valentine Jones
SSCertifiable
SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)SSCertifiable (5.9K reputation)

Group: General Forum Members
Points: 5860 Visits: 11771
You really seem to be missing the point, and your posts didn't address the specific example I posted.

I just ran this script creating the table, loading some rows into it, creating your stored proc, followed by a two executes of proc USPDeleteAnswers with my input parameters.

print '--- Create table tblUserAnswer'
create table tblUserAnswer (Id varchar(20), QuestionId int not null)
go
print '--- Load 3 rows into the table tblUserAnswer'
insert into tblUserAnswer select '1', 2 union all select '3',4 union all select '5',6
go
CREATE PROCEDURE USPDeleteAnswers
@Id varchar(50),
@QId varchar(100)
AS
DECLARE @SQL nvarchar(255)
SET @SQL = 'DELETE FROM tblUserAnswer where Id = ''' + @Id + ''' and QuestionId in (' + @QId + ' )'
PRINT @SQL ---- delete after testing
EXEC sp_executesql @SQL
go
print '--- Delete all rows from tblUserAnswer'
Execute USPDeleteAnswers @Id = 'abc', @QId = ' 1,3 ) or 1 = 1 -- '
go
print '--- Get count of remaining rows in table tblUserAnswer'
select count(*) from tblUserAnswer
go
print '--- Delete all rows from tblUserAnswer and then drop table tblUserAnswer '
Execute USPDeleteAnswers @Id = 'abc', @QId = ' 1,3 ) or 1 = 1; drop table tblUserAnswer -- '
go
print '--- Verify table tblUserAnswer has been deleted'
exec sp_help 'tblUserAnswer'
go
drop PROCEDURE USPDeleteAnswers
go
-- drop table if it still exists
if object_id('tblUserAnswer','U') is not null drop table tblUserAnswer



Notice the following results it produced:

--- Create table tblUserAnswer
--- Load 3 rows into the table tblUserAnswer

(3 row(s) affected)
--- Delete all rows from tblUserAnswer
DELETE FROM tblUserAnswer where Id = 'abc' and QuestionId in ( 1,3 ) or 1 = 1 -- )

(3 row(s) affected)
--- Get count of remaining rows in table tblUserAnswer

-----------
0

(1 row(s) affected)

--- Delete all rows from tblUserAnswer and then drop table tblUserAnswer
DELETE FROM tblUserAnswer where Id = 'abc' and QuestionId in ( 1,3 ) or 1 = 1; drop table tblUserAnswer -- )

(0 row(s) affected)
--- Verify table tblUserAnswer has been deleted
Msg 15009, Level 16, State 1, Procedure sp_help, Line 66
The object 'tblUserAnswer' does not exist in database 'tempdb' or is invalid for this operation.



The DELETE statement produced by the first call was a valid statement that resulted in deleting all rows in table tblUserAnswer because the 1 = 1 condition is always true.

The DELETE statement produced by the second call was actually able to drop the table.

As you can see, this was a really routine SQL Injection 101 attack.
Go


Permissions

You can't post new topics.
You can't post topic replies.
You can't post new polls.
You can't post replies to polls.
You can't edit your own topics.
You can't delete your own topics.
You can't edit other topics.
You can't delete other topics.
You can't edit your own posts.
You can't edit other posts.
You can't delete your own posts.
You can't delete other posts.
You can't post events.
You can't edit your own events.
You can't edit other events.
You can't delete your own events.
You can't delete other events.
You can't send private messages.
You can't send emails.
You can read topics.
You can't vote in polls.
You can't upload attachments.
You can download attachments.
You can't post HTML code.
You can't edit HTML code.
You can't post IFCode.
You can't post JavaScript.
You can post emoticons.
You can't post or upload images.

Select a forum

































































































































































SQLServerCentral


Search