views:

90

answers:

4

In our ancient Classic ASP environment, we utilize OWASP to get the password from the request object and encrypt non-alphanumeric characters. This is a first line of defense to preventing sql injection. We use other methods for full sql injection prevention.

The problem is, when we are collecting data to put together an HTTP post message and just grab the password from the user input, OWASP it and send it along. The password is therefore incorrect.

Example: Password freddie$cougar becomes freddie&36;cougar

What we ended up doing was assuming that a 50 character text field was not enough space to do much sql injection and changed the code so we didn't OWASP the password coming in. This feels a bit scary.

Is their a better way?

The code is written in vbScript.

+3  A: 

A better solution is to convert all queries to parameterized ones.

Here is a 12 year old article explaining how :)

RedFilter
Right. We do that already. But what about the case where we are just sending the data along to another vendor? That's what I'm getting at here.
crackedcornjimmy
If you are sending the data to another vendor, you shouldn't have to care about validating it. :-) In any case, they shouldn't be trusting your input either, and if they do, you shouldn't be trusting them to know what the right thing to do for their customers is...
Franci Penov
Excellent point. Thank you.
crackedcornjimmy
+3  A: 

Consider moving your SQL statements to stored procedures, and ensure that you don't use dynamic SQL within those stored procs.

Dim userPwd = Trim(Request.QueryString("userPwd"))
'--- Create and append parameter for Password
Set pwdParameter = cmd.CreateParameter("@UserPassword", ad_nVarChar, adParamInput, 50, userPwd)
cmd.Parameters.Append pwdParameter

Aside, it's definitely best to not even store the pwd in your database, but rather a salted hash.

The method above is preferred, no matter what string you're sending to your database, as it'll avoid executing directly as an adhoc statement, and will avoid SQL injection, as long as you're not using the parameter with dynamic SQL within the stored proc.

p.campbell
Note that you can parameterize your queries without using stored procedures. Use the code proposed above, but use `@UserPassword` in the code commandText. See http://blog.binarybooyah.com/blog/post/Classic-ASP-data-access-using-parameterized-SQL.aspx for discussion.
Brian
+2  A: 

Why are you sending the password in clear text? Calculate a hash value for the password and send that one. This would allow you to prevent SQL injection and to avoid man-in-the-middle type attacks.

In any case, you have to also clean up the data as it comes to your server. Even if your vbscripts does client-side validation, it would be trivial to attack your service by bypassing your script and hand-crafting a packet with malicious input in it.

Franci Penov
Whatever you do with the data at client side provides only an illusion of protection,if you transmit the data via HTTP. Remember, the response goes also unencrypted, so a MITM could just as easily spoof a login page. Your suggestion may help a bit against shared wifi snooping, though.
Piskvor
As I said, the server should not rely on any client side defenses. However, that would not be an excuse to send the password in plain text.
Franci Penov
+1  A: 

A lot of sites limit the set of characters that can be used in passwords - choose a set that is not going to cause you grief. That probably means alphanumerics, and some punctuation (comma, full stop, dash). Having suggested that, those sites annoy me - I use a rich set of characters in my passwords when given the chance to do so, and on the alphanumeric-only sites I usually end up using nonsense like 'IHateNoPunctSites' as the password.

What about shipping the password as a hex-encoded string, or a base-64 encoded string? You can then decode the string at the end, being as careful as necessary to prevent any injection without limiting the character set that is used in the password. When you need to check the password, you can ensure you are doing it cleanly using a parameterized query. Or you can hash the password with its salt before sending the query off to the password file. You should not be doing much with passwords anyway.

Jonathan Leffler