SQL Server 7 and rs.EOF problem

[lcscne]

16 sqlUser = "SELECT instructorId, schoolId, salutation, firstName, lastName, phone, email " & _
17 "FROM tblInstructors " & _
18 "WHERE (((userName)='"&Request.Form("userName")&"') AND ((password)='"&Request.Form("password")&"'))"
19
20 Set rsUser = Server.CreateObject("ADODB.RecordSet")
21 rsUser.Open sqlUser, oConn


This code runs fine as long as I key in a valid username and password. If I key in a bad username and password combo the db returns:

ADODB.Recordset error '800a0bb9'
Arguments are of the wrong type, are out of acceptable range, or are in conflict with one another.
/loginProc.asp, line 21

I'm using SQL Server v7, can anyone shed any light on this? thanks

connect string:

"Provider=sqloledb;" & _
"Data Source=(local);" & _
"Initial Catalog=GradeTrax;" & _
"User ID=sa;" & _
"Password=pword"

[buntine]

That error is caused by a number of things, normally a faulty connection string.

You should run an if statement to stop the page from executing if no records are found.

If rsUser.EOF and rsUser.BOF then
With Response
.Write ("Error: Invalid username and/or password.")
.Flush
.End
End With
End If

Regards,
Andrew Buntine.

[lcscne]

Originally posted by buntine
You should run an if statement to stop the page from executing if no records are found.


Good idea but if you will notice I'm getting the error on line 21, in actuality I have your If satement on line 22 but by then processing has stopped. Any other ideas? Whats weird to me is that it works when I key in a valid username and password. Why would it like the syntax with a valid username password combination and then reject the syntax otherwise?

[buntine]

Its definetely strange.. The error shouldnt be received until you try to iterate through your recordSet.

Try removing the parantheses from your query, they may be messing with the precedence of each operator.

sqlUser = "SELECT instructorId, schoolId, salutation, firstName, lastName, phone, email " & _
"FROM tblInstructors " & _
"WHERE userName = '"&Request.Form("userName")&"' AND password='"&Request.Form("password")&"'"

Regards,
Andrew Buntine.

[lcscne]

yo lcscne, you have a parentheses problem in your WHERE clause, see Microsoft TID 235892 (http://support.microsoft.com/default.aspx?scid=kb;en-us;Q235892)

hope this helps others.

oops, sorry Buntine, I must have been typing while you were. You nailed it, thanks.

[buntine]

Did you just answer your own question in the third-person? :confused:

[CardboardHammer]

Since you're using SQL Server, use stored procedures instead of queries and you can save yourself from some assorted PITA issues. With what you have, if you don't filter "'" from usernames and passwords, you're going to have SQL injection problems. If you use stored procedures and don't use EXEC on a string containing user input, there's no risk of SQL injection.

Injection example: username = "' OR 'a' = " password = " OR instructorId = (SELECT TOP 1 instructorId FROM tblInstructors) OR 'a' = 'z"

Which gives a query of: SELECT instructorId, schoolId, salutation, firstName, lastName, phone, email
FROM tblInstructors
WHERE (((userName)=''
OR 'a' = ') AND ((password)='
OR instructorId = (SELECT TOP 1 instructorId FROM tblInstructors)
OR 'a' = 'z'))
which is effectively: SELECT instructorId, schoolId, salutation, firstName, lastName, phone, email
FROM tblInstructors
WHERE instructorId = (SELECT TOP 1 instructorId FROM tblInstructors)
Ouch.

If an attacker knows an instructor's first and last name, they can pick who they log in as. Double ouch.

Or how about this query:SELECT instructorId, schoolId, salutation, firstName, lastName, phone, email
FROM tblInstructors
WHERE (((userName)='' AND ((password)=''))
DROP TABLE tblInstructors
SELECT ((''))
KABOOM!


Other benefits of stored procedures include:
--Better performance
--Easier reuse of SQL
--All your SQL located in one place
--Easier management of permissions (no need to give accounts ANY permissions on tables, just EXEC permissions on the appropriate stored procedures).

With a bit of creativity, you can do some amazing things with stored procedures and do them quickly.

[lcscne]

Originally posted by CardboardHammer
Since you're using SQL Server, use stored procedures instead of queries and you can save yourself from some assorted PITA issues. With what you have, if you don't filter "'" from usernames and passwords, you're going to have SQL injection problems. If you use stored procedures and don't use EXEC on a string containing user input, there's no risk of SQL injection.

CardboardDude,
your going to have to put the cookies down on the shelf for this child to get them. I'm not making bread here so I'm not sure what PITA issues I'm facing, and I'm not clear on how writing a Stored Proc can keep users from creating a custom where clause. I do have a 20 character limit set on the username and password fields so again I'm not sure what a hacker might be able to do with 20 chars. In addition I have the db design in front of me and can't get it to work, how will a hacker get at the db design when all he can see is plain html. Hey just because I'm not paranoid, doesn't mean they aren't really hacking me.

Seriously, can you explain PITA and how to create a paramaterized stored procedure that doesn't alow SQL injection. I mean the way you've explained it - seems that if we ask the user for any input, theres not much security in place to stop them from dropping the db itself whether the code reside in my asp script or in a stored procedure.

[lcscne]

to all you coding comrads. I appologize for starting this thread. I stink as a programmer, I want to pump gas for a living or mow lawn or flip burgers or something.

I had a piece of code further down that did a redirect if login failed that looped back on the same page instead of redirecting to the login page. My error message came when the page was loaded the second time with the form collection null. I hate it when computers do what you tell them to and not what you want them to.

[CardboardHammer]

HOWTO: Run SQL Server Stored Procedures from an ASP Page http://support.microsoft.com/default.aspx?scid=kb;en-us;300488

(On a side note, I haven't done classic ASP in a while, having started the projects in my current position using ASP.NET instead.)

If your length limits on the text fields aren't enforced on the server side, those limits effectively don't exist to an attacker.

Stored procedures (with the previously noted exception) prevent SQL injection because the parameters supplied are treated as values and are not parsed (the logic is already dictated). On the other hand, when you supply your query as a string, it must be parsed (because SQL Server has no idea what you expect from it before parsing the query), in which case an attacker can supply values for userName and password that cause the string to be parsed in such manner as to cause undesireable results.

Analogy:
Scenario 1 (like a query string) -- I give you instructions (which I intend to be of the form "Get a [user specified tool]") to perform written on a sheet of paper. You read the instructions and follow them. Ex. "Get a screwdriver", where "screwdriver" was specified by the user. Let's say a malicious user supplies "screwdriver and poke your eye out with it"... not good.
Scenario 2 (like a stored procedure) -- Ahead of time, you are informed that you will perform the action "Get a [user specified tool]" and that the tool name will be written on the sheet of paper. If the user supplies "screwdriver and poke your eye out with it", you won't be able to get any such tool, as no tool has that name.

With the stored procedure below, SQL Server already has the logic ready ahead of time and only needs to plug in the values of the parameters to find what you're asking for.
CREATE PROCEDURE SITE_LOGIN
@userName AS varchar(20),
@password AS varchar(20)
AS
SELECT instructorId, schoolId, salutation, firstName, lastName, phone, email
FROM tblInstructors
WHERE userName = @userName AND password = @password


The one below illustrates (I think I got it right, but you don't want to do it whether I got it right or not) a situation where an SQL injection would be possible in a stored procedure (though the parameters are strictly limited (via truncation) to 20 chars maximum, which limits the possible actions).
CREATE PROCEDURE SITE_LOGIN
@userName AS varchar(20),
@password AS varchar(20)
AS

DECLARE @qs AS varchar(200)
SET @qs = 'SELECT instructorId, schoolId, salutation, firstName, lastName, phone, email
FROM tblInstructors
WHERE userName = ''' + @userName + ''' AND password = ''' + @password + ''''

EXEC @qs


Injection is possible above because the string in @qs must be parsed before execution of the SQL contained within it.

Other PITA issues resolved by using stored procedures include:
-If you change table design, you'd have to hunt through ASP pages looking for affected queries vs. looking through a list of stored procedures (that contain SQL only and not piles of other logic).
-Permission management (especially if you use impersonation and/or there are cases where there are certain things a particular user may INSERT into a table, but they shouldn't be allowed to INSERT certain other things into that table (in which case INSERT permission on the table is too broad to provide the needed control)) is much easier, as you can ignore the underlying tables and just focus on the actions (as coded into stored procedures) that a user is allowed to perform.
-Easier reuse of SQL. You can call a stored procedure from a stored procedure. Also, you can easily find your SQL to do a copy/paste/edit to save time when you're doing something new that is similar to something you've already done.

One easy SQL injection attack is to put something like "' OR '1' = '1" in and hope that something useful falls out. Granted, some things are pretty hard to achieve by SQL injection without knowing the db structure (like dropping tables), but it's possible to get info out of simple injections that would aid in carrying out more complex injections.

[CardboardHammer]

Originally posted by lcscne
to all you coding comrads. I appologize for starting this thread. I stink as a programmer, I want to pump gas for a living or mow lawn or flip burgers or something.

I had a piece of code further down that did a redirect if login failed that looped back on the same page instead of redirecting to the login page. My error message came when the page was loaded the second time with the form collection null. I hate it when computers do what you tell them to and not what you want them to.

Don't sweat it. Most, if not all, programmers have experienced similar (or worse) mishaps. In this case, nobody died, lost an election, or got their credit history blackened. All in all, no big deal: a lesson learned at a minimal cost.