Click here to monitor SSC
SQLServerCentral is supported by Red Gate Software Ltd.
 
Log in  ::  Register  ::  Not logged in
 
 
 
        
Home       Members    Calendar    Who's On


Add to briefcase 12»»

Avoiding injection on stored procedure Expand / Collapse
Author
Message
Posted Wednesday, March 18, 2009 10:38 AM
Forum Newbie

Forum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum Newbie

Group: General Forum Members
Last Login: Wednesday, April 8, 2009 2:57 AM
Points: 2, 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
Post #678702
Posted Wednesday, March 18, 2009 11:01 AM
SSChasing Mays

SSChasing MaysSSChasing MaysSSChasing MaysSSChasing MaysSSChasing MaysSSChasing MaysSSChasing MaysSSChasing Mays

Group: General Forum Members
Last Login: 2 days ago @ 8:53 AM
Points: 605, Visits: 3,534
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))'
Post #678725
Posted Wednesday, March 18, 2009 11:52 AM


SSCertifiable

SSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiable

Group: General Forum Members
Last Login: Thursday, November 6, 2014 1:00 PM
Points: 5,333, Visits: 25,277
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
Post #678765
Posted Wednesday, March 18, 2009 12:20 PM
Forum Newbie

Forum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum Newbie

Group: General Forum Members
Last Login: Wednesday, April 8, 2009 2:57 AM
Points: 2, 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
Post #678779
Posted Wednesday, March 18, 2009 12:29 PM
Hall of Fame

Hall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of Fame

Group: General Forum Members
Last Login: Today @ 12:50 AM
Points: 3,109, Visits: 11,515
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 --  '

Post #678793
Posted Wednesday, March 18, 2009 1:22 PM


SSCertifiable

SSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiable

Group: General Forum Members
Last Login: Thursday, November 6, 2014 1:00 PM
Points: 5,333, Visits: 25,277
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
Post #678844
Posted Wednesday, March 18, 2009 2:07 PM
Hall of Fame

Hall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of Fame

Group: General Forum Members
Last Login: Today @ 12:50 AM
Points: 3,109, Visits: 11,515
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.

Post #678890
Posted Wednesday, March 18, 2009 9:48 PM


SSCertifiable

SSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiable

Group: General Forum Members
Last Login: Thursday, November 6, 2014 1:00 PM
Points: 5,333, Visits: 25,277
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
Post #679137
Posted Wednesday, March 18, 2009 9:59 PM
SSCarpal Tunnel

SSCarpal TunnelSSCarpal TunnelSSCarpal TunnelSSCarpal TunnelSSCarpal TunnelSSCarpal TunnelSSCarpal TunnelSSCarpal TunnelSSCarpal Tunnel

Group: General Forum Members
Last Login: Monday, November 3, 2014 4:30 PM
Points: 4,574, Visits: 8,366
How about this:

[dbo].[USPDeleteAnswers] 'xxx','1,3,5) or 1=1--'
Post #679139
Posted Wednesday, March 18, 2009 10:41 PM
Hall of Fame

Hall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of FameHall of Fame

Group: General Forum Members
Last Login: Today @ 12:50 AM
Points: 3,109, Visits: 11,515
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.



Post #679148
« Prev Topic | Next Topic »

Add to briefcase 12»»

Permissions Expand / Collapse