sort order via parameter-now security issue

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

General SQL Server Forums

New to SQL Server Programming

sort order via parameter-now security issue

Author

Topic

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-19 : 14:05:07

we are firing this from my asp page, what i can't work out how to do is pass a sort order via parameter to my stored procedure.
Any Help appreciated

Nigel

set conn = CreateObject("ADODB.Connection")

conn.open MM_fc_conn_STRING

sql = "EXEC Sproc_01020_Select_WebUser" & " @UserLoginName='" & Request.Form("Username") & "'"

set Recordset1 = conn.execute(sql)

SwePeso
Patron Saint of Lost Yaks

30421 Posts

Posted - 2007-03-19 : 14:08:19

Add a second parameter

sql = "EXEC Sproc_01020_Select_WebUser @UserLoginName = '" & Request.Form("Username") & "', @SortOrder = '" & Request.Form("SortOrder") & "'"

Peter Larsson
Helsingborg, Sweden

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-19 : 14:12:54

but how do i set up the stored procedure to except this parameter as a means to sort the data, I know how to setup normal select parameters as i am using loads of these, but how do i setup one that will change the sort order of the data returned.

SwePeso
Patron Saint of Lost Yaks

30421 Posts

Posted - 2007-03-19 : 14:16:08

ORDER BY CASE...

or

IF @SortOrder = 1
select ... order by col1

If @SortOrder = 2
select ... order by col2

Peter Larsson
Helsingborg, Sweden

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-19 : 14:41:30

How would i implement
IF @SortOrder = 1
select ... order by col1

If @SortOrder = 2
select ... order by col2

this into the following sql i want to be able to sort multiple fields and give the web page the choice whether its desc or asc

ALTER PROCEDURE [dbo].[Sproc_04010_BF2_PerRoundEvents_Select]

@TourTypeId int = Null,
@TourName varchar(max) = '%',
@TourId int = Null,
@MemberId int = Null,
@PlayerID int = Null,
@PlayerName varchar(max) = '%',
@MapName varchar(max) = '%',
@StartDateStr varchar(8) = '%',
@EndDateStr varchar(8) = '%'

AS
BEGIN
SELECT Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.ID,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.TourTypeId,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.TourName,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.TourId,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.MapReference,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.MemberId,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.PlayerID,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.PlayerName,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.Score,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.MapName,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.FirstScoreEventTime,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.LastScoreEventTime,
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.DateOfRoundStr, Tbl_01002_CountryFlags.FlagCountry,
Tbl_01002_CountryFlags.FlagCountryAbbreviation, Tbl_01002_CountryFlags.FlagPicLocation
FROM Tbl_01002_CountryFlags INNER JOIN
Tbl_01010_Users ON Tbl_01002_CountryFlags.FlagCountry = Tbl_01010_Users.UserCountry RIGHT OUTER JOIN
Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound ON
Tbl_01010_Users.MemberId = Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.MemberId

Where
(@TourTypeId is Null Or TourTypeId = @TourTypeId) And
(@TourName = '%' Or TourName = @TourName) And
(@TourId is Null Or TourId = @TourId) And
(@MemberId is Null Or Tbl_02012_BF2_Scores_PerTourType_PerTour_PerPlayer_PerRound.MemberId = @MemberId) And
(@PlayerID is Null Or PlayerID = @PlayerID) And
(@PlayerName = '%' Or PlayerName = @PlayerName) And
(@MapName = '%' Or MapName = @MapName) And
(@StartDateStr = '%' Or DateOfRoundStr >= @StartDateStr) And
(@EndDateStr = '%' Or DateOfRoundStr <= @EndDateStr)

SwePeso
Patron Saint of Lost Yaks

30421 Posts

Posted - 2007-03-19 : 14:57:24

Have you considered using Table Aliases?

Peter Larsson
Helsingborg, Sweden

jezemine
Master Smack Fu Yak Hacker

2886 Posts

Posted - 2007-03-19 : 16:19:08

quote:
Originally posted by nobby

sql = "EXEC Sproc_01020_Select_WebUser" & " @UserLoginName='" & Request.Form("Username") & "'"

set Recordset1 = conn.execute(sql)

this code is vulnerable to sql injection. I wouldn't put this into production unless you don't mind if a criminal steals your data.

www.elsasoft.org

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-19 : 21:05:26

Jezemine
I am new to sql and you say my code is vunerable could you tell me in what way its vunerable and also how to make it safer. Bearing in mind this is only a select query to desplay the data on a web page.
Also can you help with my original question

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-19 : 22:22:24

Thanks for help i have gone down this root and all appears fine
Can anyone help with the security issue Jezemine mentioned as i am very worried about my code now

order by
case when @OrderByOption = 1 Then TourTypeId end asc,
case when @OrderByOption = 2 Then TourTypeId end desc,
case when @OrderByOption = 3 Then DateOfRoundStr end asc,
case when @OrderByOption = 4 Then DateOfRoundStr end desc

jezemine
Master Smack Fu Yak Hacker

2886 Posts

Posted - 2007-03-19 : 23:22:01

Here's a nice little video that shows you what one can do with a site that's vulnerable to sql injection.

http://www.rockyh.net/AssemblyHijacking/AssemblyHijacking.html

credit to pootle_flump for the link from this thread: http://www.dbforums.com/showthread.php?t=1616096

The way to make your code safe against injection is to NEVER create a sql statement by concatenating user input and then executing it. always use parameterized queries, even if it's an ad hoc sql statement and not a sproc call. in .net, that means always using the SqlCommand.Parameters collection to pass params, not concatenating into strings as you are doing.

consider what might happen if someone passed this value for your Username param on the querystring:

Username=joe'; drop database accounts --

www.elsasoft.org

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-19 : 23:52:34

jezemine,
Thanks for the warning i watched the video and its frightening. I am very new to sql and learning as i go i didn't really understand your explanation on how to do this safely could you give me an example of how to do similiar to this safely. in the example the field UserLoginName is not the database admin login but a user login in the database

sql = "EXEC Sproc_01020_Select_WebUser" & " @UserLoginName='" & Request.Form("Username") & "'"

Your help and advice is much appreciated
Nigel

quote:
Originally posted by jezemine

Here's a nice little video that shows you what one can do with a site that's vulnerable to sql injection.

http://www.rockyh.net/AssemblyHijacking/AssemblyHijacking.html

credit to pootle_flump for the link from this thread: http://www.dbforums.com/showthread.php?t=1616096

The way to make your code safe against injection is to NEVER create a sql statement by concatenating user input and then executing it. always use parameterized queries, even if it's an ad hoc sql statement and not a sproc call. in .net, that means always using the SqlCommand.Parameters collection to pass params, not concatenating into strings as you are doing.

consider what might happen if someone passed this value for your Username param on the querystring:

Username=joe'; drop database accounts --

www.elsasoft.org

jezemine
Master Smack Fu Yak Hacker

2886 Posts

Posted - 2007-03-20 : 12:56:52

are you using .net? if so have a look at these examples:

http://msdn2.microsoft.com/en-us/library/system.data.sqlclient.sqlcommand.parameters.aspx

www.elsasoft.org

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-20 : 13:38:26

No we are not using .net and have come up with this , is this secure

Set Recordset1_cmd = Server.CreateObject ("ADODB.Command")

Recordset1_cmd.ActiveConnection = conn_STRING

Recordset1_cmd.CommandText = "dbo.MyStoredProcedure"

Recordset1_cmd.Prepared = true

Recordset1_cmd.Parameters.Append Recordset1_cmd.CreateParameter("@MyParam", 200, 1, -1, "MyValue")

Set Recordset1 = Recordset1_cmd.Execute

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-20 : 13:38:26

jezemine
Master Smack Fu Yak Hacker

2886 Posts

Posted - 2007-03-20 : 14:56:15

yes, this is the way to do it when using old school ADO.

posting each reply just once is sufficient btw.

www.elsasoft.org

jsmith8858
Dr. Cross Join

7423 Posts

Posted - 2007-03-20 : 15:36:29

Also, note that security isn't the only reason to use parameters:

http://weblogs.sqlteam.com/jeffs/archive/2006/07/21/10728.aspx

that article specifically covers .NET, but it applies to old school ADO as well. In addition to security, when you concatenate together strings like that, you deal with delimiting, escaping, date formats, and other issues. Keep it simple, use best practices, use parameters! Everyone wins!

- Jeff
http://weblogs.sqlteam.com/JeffS

nobby
Yak Posting Veteran

58 Posts

Posted - 2007-03-20 : 17:19:05

jezemine Thanxs for help and advice

quote:
Originally posted by jezemine

yes, this is the way to do it when using old school ADO.

posting each reply just once is sufficient btw.

www.elsasoft.org

jezemine
Master Smack Fu Yak Hacker

2886 Posts

Posted - 2007-03-21 : 02:06:18

welcome!

www.elsasoft.org

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources