The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

SQL Injection advice

I am building a site where users need to login. To protect myself against SQL injection attacks, this is done through a stored procedure (see below) -- and not by building up a SQL string in the ASP code.

Besides this, is there anything else I should do to protect myself against SQL injection attacks. Also, are there any changes I should make to the stored procedure below to make it safer ?

Technology used:
SQL Server 2000
ASP.NET 1.1 (VB)

Thanks,
James

CREATE proc Members_Login
  @EmailAddress varchar(100),
  @Password varchar(50),
  @MemberID int output
as

select @MemberID = MemberID
from Members
where EmailAddress = @EmailAddress
and [Password] = @Password

if @MemberID is null
begin
  raiserror('Invalid email address or password', 16, 1)
  return 50000
end

return 0
James Bucham Send private email
Friday, November 24, 2006
 
 
The stored procedure is safe enough.  But are the passwords?  It looks like you might be performing a plaintext password comparison for the login procedure.  If the database were ever compromised the passwords of all users would be available in plaintext.

You might want to salt and hash the password in order to protect the passwords themselves.
Jared
Friday, November 24, 2006
 
 
+1 for "salt and hash the password"

Storing passwords in the clear is generally a bad idea as it means that anyone who gets read access to the database can then impersonate "real" users. Whether this is an issue for your system is something that you have to judge.

Another thing that you may want to do is use a different database account for login that only has access to execute this one stored procedure.
Arethuza Send private email
Saturday, November 25, 2006
 
 
Also, FYI, using a stored procedure isn't necessary to prevent SQL injection.  A plain text query but using parameters for user-inputs will provide the same protection.  (There are other potential benefits too but mentioning those could start a holy war... :)
James Brechtel Send private email
Saturday, November 25, 2006
 
 
That looks reasonable.

I'd also recommend strongly signed assemblies, and that the site uses a limited functionality login. There should be no way that the website could execute xp_cmdshell, or any stored procs starting with xp.

From this older post on this forum:
http://discuss.joelonsoftware.com/default.asp?design.4.411839.14

I recommend reading the following book:
http://www.amazon.com/exec/obidos/ASIN/0321320735/ref=nosim/librarything-20

and the following demo was pretty scary too:
http://www.rockyh.net/AssemblyHijacking/AssemblyHijacking.html
Peter
Saturday, November 25, 2006
 
 
I'm confused by the comments about salting and hashing the password.  My stored procedures tend to look similar to this example, but I only store hashes of passwords and do the hashing outside of the stored procedure, so the password parameter that gets passed in is already hashed.

Are you suggesting doing the hashing in the stored procedure itself?  The stored value is still the hash, so how would that be different or more secure?  If someone gets the database, they still get only the hashed values.

Or perhaps the discussion is just that, from the example given, there's no way to know what the OP is doing.

Curious, thanks.
JP Send private email
Monday, November 27, 2006
 
 
I'd say, by doing the hashing in the stored procedure, it is encapsulated & centralized so that calling applications does not need to worry about how the password is hashed.
Cory R. King
Monday, November 27, 2006
 
 
Ok, but that's not a security reason.  I already have a security layer; my security code is encapsulated and centralized so calling apps don't need to worry about it.  The security layer knows about hashes and salts and -- most importantly -- key management.  I'd argue that none of that logic belongs in a database layer, but that's me.  Unless there's a good security reason for it, that is.
JP Send private email
Monday, November 27, 2006
 
 
Salting is a good precaution, and it doesn't take that long to implememt.

Just make sure you know what salting is for... it's solely to prevent dictionary attacks in the even of a database compromise. The attacker can't a) See if two users have the same password or b) Run a dictionary over the hashes to find common words.

Basically you create a random salt when a user is created, and append that to the password when you hash it.

If you implement salting you would then need to move the hashing code into the stored procedure.
Mike Weller Send private email
Monday, November 27, 2006
 
 
Can someone show me some code (or point me to an exmaple) that does the hashing in the stored procedure?

(I am using SQL Server 2000)
James Bucham Send private email
Tuesday, November 28, 2006
 
 
You don't need to do the hashing in the stored procedure. You can do it in the client code and just pass the resulting hash to the stored procedure for comparison.
anon
Tuesday, November 28, 2006
 
 
About salting.

Why does the typical method use random characters as the salt, causing you to carry those characters around.

Instead, what about using the username as the salt in that stored = hash( concat( username, password ) )

Dictionay attacks still are folied because you would need to build a dictionary for each username. Unless building that dictionary is easy, but then it would be even easier using a two character salt.
i remember man crypt
Thursday, November 30, 2006
 
 
A two-character salt doesn't really add anything.  It will lower the chance that two copies of the same password will hash to the same value, but that's about it.  If your username/password combination is long enough, it would help, but it's no match for a long random sequence.

With the advent of rainbow tables, I think you should consider any hashed password less than about 20 characters to be insecure.  I know that you can openly buy rainbow tables for several hash algorithms up to 14 characters.  Because of how they work, I don't know that a stronger hash is any less vulnerable; it just means that someone had to have already built the table for that particular hash.

Anyway, I know that for password-based encryption, the RSA recommends salt lengths of no less than 64 bits.  I don't know what their exact recommendation is for a one-way hash.

See http://www.rsasecurity.com/rsalabs/node.asp?id=2127 for "PKCS #5 v2.0 Password-Based Cryptography Standard."
JP Send private email
Saturday, December 02, 2006
 
 
"Also, FYI, using a stored procedure isn't necessary to prevent SQL injection.  A plain text query but using parameters for user-inputs will provide the same protection.  (There are other potential benefits too but mentioning those could start a holy war... :) "

Right on the money.  BTW, that is my favorite holy war, but I won't start it here, I'll focus on how to prevent SQL injection rather than what stored procedures do or don't do outside the context of SQL injection.

I would add that in addition to a stored procedure not being the only way to prevent SQL injection, I would also caution that using a stored procedure actually does nothing to prevent SQL injection.  It's how you call it that counts, not what it is.

You said that you are calling a stored procedure, not biulding the SQL string in the ASP code.  Well, are you calling the sp like this? --

cmd.CommandText = "EXEC Members_Login '" + email + "', '" + pwd + "', " + memberID.toString();
cmd.ExecuteNonQuery();

If so, the you are vulnerable to SQL injection.  Yes ladies and gentlemen, this would be an example of using a stored procedure and still being vulnerable to SQL injection.  If you are calling it like this:

cmd.Parameters["@EmailAddress"].Value = email;
cmd.Parameters["@Password"].Value = pwd;
cmd.Parameters["@MemberID"].Value = memberID;
cmd.ExecuteNonQuery();

Then, you are protected from SQL injection.  An important note is that if your weren't using a stored procedure, but instead created a command like this:

cmd.CommandText = @"
  select @MemberID = MemberID
  from Members
  where EmailAddress = @EmailAddress
  and [Password] = @Password

  if @MemberID is null
  begin
    raiserror('Invalid email address or password', 16, 1)
    return 50000
  end
";
cmd.Parameters.Add("@EmailAddress", SqlDbType.VarChar, 100);
cmd.Parameters.Add("@PasswordD", SqlDbType.VarChar, 50);
cmd.Parameters.Add("@MemberID", SqlDbType.Int);

It would have EXACTLY the same protection against SQL injection as the stored procedure version.  The SQL injection protection isn't done in the database server, it is done in the .Net framework in the Parameter class.

Anyone telling newbies that stored procedures protect against SQL injection is doing them a disservice.  Those newbies are going to go out and string concatenate a stored procedure call and get hacked via SQL injection.
JSmith Send private email
Monday, December 11, 2006
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz