The Daily WTF: Curious Perversions in Information Technology

2007-01-26 Reply Admin

The real WTF is that so many WTFs are just stupid "SELECT * FROM table" SQL statements. Where the hell is the real WTF code nowadays?

akatherder · 2007-01-26 Reply Admin

Xaroth:
A friend of mine did a quick count on the database of his semi-popular website to see how many passwords were '12345' (he was using a non-salted hash for passwords at the time, so getting the appropriate hash for that password was trivial). As it turns out, over 5% of his users had that as their password, and another 2-3% had '123456' as their password. One can only assume that that carries over into the rest of the world, so getting access to an online account might be as easy as just getting 20 or so usernames and trying them 'til one works. *shudder*

That depends on what kind of site it is. When a site makes me sign up for something I just want to see one time, I try to make the username and password as easy to guess as possible so people can guess it and harass people on the site.

2007-01-26 Reply Admin

anon:
Personally I'd do something like this:
In the DB store the username, and the hashed password.

Client requests salt.

Server generates random salt, sends it to the client and remembers it.

Client sends username/hash(salt+hash(password)) to the server.

Server selects username/password from the database.

Server hashes the password with the random salt, then compares them.

That should be reasonably secure. Even if the hashed password is sniffed en-route, it's useless, and the reason for hashing twice is so you can store the password in the database and use a dynamic salt.

(Note: I'm assuming a web interface here)

The problem with that is reliability. You can never rely on the client to actually work properly (or have JS enabled at all), thus you'll end up with legitimate users who can't get in. Tunneling through SSH (https) is really the only worthwhile way to avoid sending plaintext passwords from client to server. With anything else you'll just end up running into security and/or reliability problems.

2007-01-26 Reply Admin

Poltras:
Pap:

Or tell the client that their password was correct but not their username.

;]
Why would you want that? So that I can profile every username in your database and then use birthday attack? "Invalid pass or user, enter again you dumbass, you'll never know if the user exists" at least, please.

CAPTCHA: pirates. yarrrrrh

You're missing the joke in the subtle wording of what he said:

He's saying tell the user when their password was correct, but not the username. Think about it: if the username is wrong, how can the program possibly know whether or not the password is correct?

2007-01-26 Reply Admin

Abscissa:
anon:
Personally I'd do something like this:
In the DB store the username, and the hashed password.

Client requests salt.

Server generates random salt, sends it to the client and remembers it.

Client sends username/hash(salt+hash(password)) to the server.

Server selects username/password from the database.

Server hashes the password with the random salt, then compares them.

That should be reasonably secure. Even if the hashed password is sniffed en-route, it's useless, and the reason for hashing twice is so you can store the password in the database and use a dynamic salt.

(Note: I'm assuming a web interface here)

The problem with that is reliability. You can never rely on the client to actually work properly (or have JS enabled at all), thus you'll end up with legitimate users who can't get in. Tunneling through SSH (https) is really the only worthwhile way to avoid sending plaintext passwords from client to server. With anything else you'll just end up running into security and/or reliability problems.

(Sorry, didn't login and can't edit) I was assuming by "client" and "server" you meant web client/server. If you meant SQL client/server then nevermind.

Red5 · 2007-01-26 Reply Admin

Pass the towel. I just puked on my keyboard. 1.Inline SQL 2.Select * 3.improper while loop 4.WHERE clause...what WHERE clause? 5.God help the poor bastard if this in a web interface

2007-01-26 Reply Admin

foofoodyne:
Killen:
Aaron:
The Real WTF™®? is that he didn't use a salt.

Actually, there's no proof that he didn't use a salt in this example.

Second that vote.

I'll grant you that it's not impossible, but I'll bet you dollars to donuts that the calling code looks like:

SomeObject.Authenticate(
 txtUserName.Text, 
 FormsAuthentication.HashPasswordForStoringInConfigFile(txtPassword.Text, "SHA1")
);

Most likely stuck directly in a "Click" event handler.

On an aside, I can't believe the number of people here who clearly have no understanding of what a salt is for. Generating a random hash during authentication does not help prevent a dictionary attack. That's just a half-baked implementation of a security context token, and it doesn't work at all when you have no control over the "client" (which you don't on the web).

Just hash once. Further hashing actually makes you less secure because every hash carries the probability of collisions, and you're increasing that probability with each successive hash. Adding a random salt doesn't help, because anyone who can see the hashes can also see the salt. If you're worried about traffic sniffing, they came up with a solution for that years ago, it's called SSL!

2007-01-26 Reply Admin

akatherder:
That depends on what kind of site it is. When a site makes me sign up for something I just want to see one time, I try to make the username and password as easy to guess as possible so people can guess it and harass people on the site.

Try http://www.bugmenot.com first. :)

danixdefcon5 · 2007-01-26 Reply Admin

Some apps I've seen made by lazy users store passwords in plaintext.

Oh ... that decryptMd5() is possible, thanks to MySQL. I don't know how they implement "md5", but md5(md5('doofus')) will return 'doofus' in MySQL. Kind of defeats the purpose altogether...

Ahh, but The Real WTF(tm) is that they should be using LDAP for authentication. ;)

2007-01-26 Reply Admin

Abscissa:
(Note: I'm assuming a web interface here)
The problem with that is reliability. You can never rely on the client to actually work properly (or have JS enabled at all), thus you'll end up with legitimate users who can't get in.

Actually, the outer hashing with salt is already covered by HTTP digest authentication. The inner hashing can be performed on the server, which is pretty much equivalent from a security point of view.

Anyhow, you are right about the benefits of SSL. I usually use it because if you need secure authentication, you most likely need the confidentiality and message integrity features offered by SSL as well. What use is a secure bank login if account information and transactions are transmitted in plaintext and subject to normal network eavesdropping and injection attacks?

Nitpicking: SSH is shell over SSL. HTTPS is HTTP over SSL, not HTTP over SSH.

triso · 2007-01-26 Reply Admin

hk0:
Wonderful reply. Excellent commenter. Would read again! A++++!!!!

Are you having an eBay flashback?

2007-01-26 Reply Admin

Red5:
Pass the towel. I just puked on my keyboard. 1.Inline SQL 2.Select * 3.improper while loop 4.WHERE clause...what WHERE clause? 5.God help the poor bastard if this in a web interface

Superfluous if-else.

authenticated =
       source["user"].ToString() == username
  &&   source["pass"].ToString() == passhash;

would have been sufficient.

Superfluous calls to ToString().

authenticated =
       username.Equals(source["user"])
  &&   passhash.Equals(source["pass"]);

marvin_rabbit · 2007-01-26 Reply Admin

Xaroth:
A friend of mine did a quick count on the database of his semi-popular website to see how many passwords were '12345' (he was using a non-salted hash for passwords at the time, so getting the appropriate hash for that password was trivial). As it turns out, over 5% of his users had that as their password, and another 2-3% had '123456' as their password. One can only assume that that carries over into the rest of the world, so getting access to an online account might be as easy as just getting 20 or so usernames and trying them 'til one works. *shudder*

<Assume that this is the obligatory '<i>changing the combination on my luggage' joke.>

webzter · 2007-01-26 Reply Admin

Aaron:
webzter: You call SecurityUtility.Encrypt(password) with no additional parameters. Where exactly is the salt? Are you just slapping some characters at the end of the hash itself (which would be useless), or am I missing something?

SecurityUtility.Encrypt is another one of my methods. It handles the salted hash. (mmm, salted hash browns, I'm hungry now).

And, for this particular instance, since, like I said, I'm not too concerned with the data actually being compromised, I'm just fine with the salt being baked so that it's just unique to my site.

2007-01-26 Reply Admin

J:
AT:
J:
Why add a WHERE clause at all? There is no good reason to send the password back to the web server, hashed or not. A stored proc that checks the login and returns a true / false would eliminate the issue entirely.

Do you think that makes the system more secure? You do realize that the hashed password already must go from the user's computer to the Web server, then from the Web server to the database right?

Do you think it makes the system less secure? What if the user enters the wrong password? What if the username isn't even valid? Why send the correct password back to the web server when it can do the check at the database?

Or, instead of going all the way to stored proc land:

Select Count(*) Where ....

The only advantage I can see for a stored proc is that with that approach you could not allow the server to access the user/password table at all (only allow external access to the stored proc), which would eliminate the possibility of someone stepping in and pretending to be your server. 'Course, if that's a possibility, you have many more serious security issues to deal with in the rest of your app too, and just returning a boolean to the (bogus) web server and expecting them to act honorably with it is beyond mere silliness.

2007-01-26 Reply Admin

danixdefcon5:
Oh ... that decryptMd5() is possible, thanks to MySQL. I don't know how they implement "md5", but md5(md5('doofus')) will return 'doofus' in MySQL. Kind of defeats the purpose altogether...

You've confirmed this?

2007-01-26 Reply Admin

danixdefcon5:
Oh ... that decryptMd5() is possible, thanks to MySQL. I don't know how they implement "md5", but md5(md5('doofus')) will return 'doofus' in MySQL. Kind of defeats the purpose altogether...

You've confirmed this?

triso · 2007-01-26 Reply Admin

danixdefcon5:
... Oh ... that decryptMd5() is possible, thanks to MySQL. I don't know how they implement "md5", but md5(md5('doofus')) will return 'doofus' in MySQL. Kind of defeats the purpose altogether...

Sorry but only a 'doofus' would consider that statement to be true unless rot13 is considered to be identical to md5, or maybe in Access.

triso · 2007-01-26 Reply Admin

danixdefcon5:
... Oh ... that decryptMd5() is possible, thanks to MySQL. I don't know how they implement "md5", but md5(md5('doofus')) will return 'doofus' in MySQL. Kind of defeats the purpose altogether...

Sorry but only a 'doofus' would consider that statement to be true unless rot13 is considered to be identical to md5, or maybe in Access.

2007-01-26 Reply Admin

danixdefcon5:
Some apps I've seen made by lazy users store passwords in plaintext.
Oh ... that decryptMd5() is possible, thanks to MySQL. I don't know how they implement "md5", but md5(md5('doofus')) will return 'doofus' in MySQL. Kind of defeats the purpose altogether...

Ahh, but The Real WTF(tm) is that they should be using LDAP for authentication. ;)

I should just bitch slap you back to the dark ages. If you knew anything at all about MD5, which you obviously don't, you'd realize it is impossible to "decrypt" a MD5 hash. Hash. Hash. Repeat to yourself. Hash. It's not just a drug.

And yes, I realize it's not quite impossible. But that's because MD5 is not totally secure.

2007-01-26 Reply Admin

The real WTF is a unit test obviously didn't exist for the unit test. Everyone knows you should unit test your unit tests, otherwise how can you be sure your code is correct?

2007-01-26 Reply Admin

I think I know "Karl". I used to have a developer working for me who wrote this kind of code (slurp a whole DB table into memory and scan it in his code).

2007-01-26 Reply Admin

Da' Man:
BTW: I give that bloke one thumb up for using hashes rather than sending plain passwords over the network (you don't see that kind of good thinking so often, believe me!)

What good does it do to pass a hash over the network instead of the plain text password? Either way, a snooper has what he need to authenticate. I mean, the server's just going to compare the hash from the client with the hash in the database, right? It is actually worse because getting the users table gets all the "passwords" with no cracking required.

2007-01-26 Reply Admin

The real wtf is 120 or so comments about password hashing. The password is already hashed before it gets to this function.

To the people who are commenting about weak passwords, the user register, should not even allow password like the ones you guys have suggested.

The problem wasn't the unit test itself, it was the amount of test data that was tested.

Anyway enough of that crap, a better solution would be:

public void Authenticate( string username, string passhash ){
   SqlDataReader source = _Database.Query("SELECT Name FROM users" + 
      " WHERE user='" + escape(username) + 
     "' AND pass='" + escape(passhass) "' ;");
   return source.HasRows;
}

2007-01-26 Reply Admin

Anonymous Coward:
Ice^^Heat:
Damn,
50% of the WTF's out there concern the WHERE clause don't they???!

At least that one was resistant to SQL injection... ;-)

"True security" is achieved by blocking everyone out except "the chosen one". :P

//jotted down

2007-01-27 Reply Admin

Matthew:
What good does it do to pass a hash over the network instead of the plain text password? Either way, a snooper has what he need to authenticate. I mean, the server's just going to compare the hash from the client with the hash in the database, right? It is actually worse because getting the users table gets all the "passwords" with no cracking required.

Yup - if you just send, for example, md5(password) in cleartext everywhere, then congratulations - you've just made your system as insecure as plaintext passwords, as the md5 hash has become the password.

If you have a secure comms channel, e.g. SSL/TLS, it's easiest to just send the plaintext password over that secure channel, and use a salted hash on the server end and in the DB.

If you have an insecure channel, something like CRAM-MD5 or similar is a good idea (although this requires the server to have access to the decrypted password - which can be a problem in itself):

Server sends $random_string
Client generates md5($random_string + $password), sends
Server generates md5($random_string + $password), compares to what client sends.

2007-01-27 Reply Admin

You might want to look into a new line of work if you don't find this code absolutely painful to look at or think about. The person who wrote it was/is an idiot with no clue.

Selecting the entire table (rather than just the correct username's row).
Iterating through every entry to test the username/password, giving O(N) instead of O(1) behavior.
Not breaking if a correct username is found, therefore only working if the matching username is the last one returned by the database.

2007-01-27 Reply Admin

Pap:
Anonymous:
Doing "SELECT * FROM users WHERE password = 'password' is still very poor practice because you're getting all users who have that password, now you get to loop over them on the web server to find the one with the right username. Worse like the original posted said, using SELECT * is almost, always a bad idea.
The database server is optimized to find things like this for you, there's almost no reason to ever loop over a record set searching for something.

"SELECT count(*) FROM Users WHERE password = 'hashed-password'" and I'd be using bind parameters as well.

You never check the username. That would log you in for guessing any user's password correctly without even knowing the username.

No more of this count(*) bs:

"SELECT TRUE FROM users WHERE username = ? AND password = ?" (parameterized).

Replace "TRUE" with "userid" for efficiency if you're into that sort of thing :D

Oh Dammit. I copy pasted the SQL line from something else and lost the rest of the WHERE clause.

Good catch.

2007-01-27 Reply Admin

AT:
Matthew:
What good does it do to pass a hash over the network instead of the plain text password? Either way, a snooper has what he need to authenticate. I mean, the server's just going to compare the hash from the client with the hash in the database, right? It is actually worse because getting the users table gets all the "passwords" with no cracking required.

Yup - if you just send, for example, md5(password) in cleartext everywhere, then congratulations - you've just made your system as insecure as plaintext passwords, as the md5 hash has become the password.

with a good password(which should be enforced the user management), it is vary vary hard(the only method to crack the password is a brute-force attack) and time consuming to crack the password (which should be change constantly).

AT:
If you have a secure comms channel, e.g. SSL/TLS, it's easiest to just send the plaintext password over that secure channel, and use a salted hash on the server end and in the DB.
If you have an insecure channel, something like CRAM-MD5 or similar is a good idea (although this requires the server to have access to the decrypted password - which can be a problem in itself):

Server sends $random_string

Client generates md5($random_string + $password), sends

Server generates md5($random_string + $password), compares to what client sends.

There is still a possiblity of a man-in-the-middle attack. More for the CRAM-MD5, but still there is a threat for ssl.

2007-01-27 Reply Admin

Nitpicking: SSH is shell over SSL. HTTPS is HTTP over SSL, not HTTP over SSH.

Nitpicking your nitpick: SSH (the protocol) does not use SSL (the protocol).

cklam · 2007-01-28 Reply Admin

Aaron:
foofoodyne:
Killen:
Aaron:
The Real WTF™®? is that he didn't use a salt.

Actually, there's no proof that he didn't use a salt in this example.

Second that vote.
I'll grant you that it's not impossible, but I'll bet you dollars to donuts that the calling code looks like:
SomeObject.Authenticate(
 txtUserName.Text, 
 FormsAuthentication.HashPasswordForStoringInConfigFile(txtPassword.Text, "SHA1")
);
Most likely stuck directly in a "Click" event handler.

On an aside, I can't believe the number of people here who clearly have no understanding of what a salt is for. Generating a random hash during authentication does not help prevent a dictionary attack. That's just a half-baked implementation of a security context token, and it doesn't work at all when you have no control over the "client" (which you don't on the web).

Just hash once. Further hashing actually makes you less secure because every hash carries the probability of collisions, and you're increasing that probability with each successive hash. Adding a random salt doesn't help, because anyone who can see the hashes can also see the salt. If you're worried about traffic sniffing, they came up with a solution for that years ago, it's called SSL!

SSL: Second that.

Salt and then hash. That is enough. That is application security, which should be implemented.

If you worried about network security, then you use secure network mechanisms.

If you are worried about client (PC) security: Often, you do not have any control over this. If you have your user accessing your application from non-secured and/or compromised clients, then that's that - case closed: access to your application for the users in question is compromised.

That's bad enough - but the problem can be mitigated depending on the nature of the application: i. e. for e-banking web apps two-factor (or even three-factor authentication) should be implemented (at least) for performing actual transcations.

One way to do this are one-time-passwords (in Germany they are called TAN (short for german Transaktionsnummer - in english transaction number). You get a pad (called a block)in a secure tamperproof envelope sent to you by mail from your bank which contains about one hundred transactions numbers. Any new block (with any transaction number) contained therein) needs to activated by the end user in the banking app using an unused transaction number from the current TAN block. Doing this successfully deactivates the current TAN block and at the same time activates the new TAN block.

Before, you as an end user could select at your will which TAN from the current TAN block to use for a transaction. Nowadays (presumably because users were running out TAN on their current block or writing them down elesewhere/storeing them on PDAs or whatever) the banking apps requests you provide a TAN specified by row and cloumn on the original TAN block paper.

Of course, this is not foolproof: The biggest WTF that you can do with this scheme is to store your banking app login info together with the TAN block. That is why the banking apps in Germany do not require TANs for login authentication.

2007-01-28 Reply Admin

It's a Feature:
Pon:
The REAL WTF is that he's persisting hashes.

Remind me not to hire you when I need a security program written.

The alternative is to store passwords in plain text--which of course means that anyone with access to the database can find out anyone's password in a heartbeat--plus probably means that password may have to travel a network in plain text. In other words, rather useless security.

Had you read my next comment...

2007-01-28 Reply Admin

Actually the REALL issue here is writing code like this in the first place. What experienced coder (even on that sucks) pulls every record in a table when the conditions for the where clause are in his hand? That's a noob error - who put the noob on security?

2007-01-28 Reply Admin

J:
SnapShot:
Pon:
The REAL WTF is that he's persisting hashes.

Why's that? I persist hashes instead of the plaintext password all the time. But I'd be interested to know why that's not recommended (other than the inability to email passwords to the user if they forget theirs; I generally generate a new random password instead.)

Ditto to that. I never store the actual password, and I get quite upset with organizations that do. Especially when they have some random idiot in a call center asking me to repeat my password over the phone to them to verify who I am! WhyTF are they allowed to see it! WhoTF's bright idea was that!

In such cases the password that the call center employee is asking you for is your ACCOUNT password, not your LOGIN password, and as such, it must be stored unencrypted so the service rep can verify the account password provided over the phone. (unlike your login password).

Such account passwords are a good idea, otherwise it is too easy for someone to call the company concerned and make unauthorized changes to your account or gather personal information about you.

Generally I do not like to deal with companies that do not use an account password, having had someone obtain a phone in my name without my knowledge once.

That said, plenty of sites do not hash the password before it is sent to the webserver. It can be achieved with javascript, but what if it is not enabled in the clients browser? Login will fail. It is foolish to rely on javascript. The only way to ensure that a password is reliably encrypted before transmission is to use an SSL webserver.

The real WTF here is that so many posters here think they know what they are talking about.

And yes, the author of the code posted is obviously pretty clueless.

2007-01-28 Reply Admin

AC:
In such cases the password that the call center employee is asking you for is your ACCOUNT password, not your LOGIN password, and as such, it must be stored unencrypted so the service rep can verify the account password provided over the phone. (unlike your login password).

Perhaps the call centre person could type in the password you read to them over the phone? Then the system would do a normal password check, and a plain-text password doesn't need to be stored... Just an idea.

Or even do what HSBC in the UK does -- only ask for particular digits of the password, so it never really goes in one chunk. (Horrible to use, but seems slightly more secure.)

Hawk777 · 2007-01-28 Reply Admin

brendan:
AT:
Matthew:
What good does it do to pass a hash over the network instead of the plain text password? Either way, a snooper has what he need to authenticate. I mean, the server's just going to compare the hash from the client with the hash in the database, right? It is actually worse because getting the users table gets all the "passwords" with no cracking required.

Yup - if you just send, for example, md5(password) in cleartext everywhere, then congratulations - you've just made your system as insecure as plaintext passwords, as the md5 hash has become the password.

with a good password(which should be enforced the user management), it is vary vary hard(the only method to crack the password is a brute-force attack) and time consuming to crack the password (which should be change constantly).

AT:
If you have a secure comms channel, e.g. SSL/TLS, it's easiest to just send the plaintext password over that secure channel, and use a salted hash on the server end and in the DB.
If you have an insecure channel, something like CRAM-MD5 or similar is a good idea (although this requires the server to have access to the decrypted password - which can be a problem in itself):

Server sends $random_string

Client generates md5($random_string + $password), sends

Server generates md5($random_string + $password), compares to what client sends.

There is still a possiblity of a man-in-the-middle attack. More for the CRAM-MD5, but still there is a threat for ssl.

Two of your points are untrue. Regarding hashes:

If a client authenticates by sending a username+password, the attacker simply sniffs the username+password and repeats them.
If a client authenticates by sending username+md5(password), the attacker simply sniffs the username+md5(password) and repeats them. THIS IS NO HARDER!

Also, SSL is secure against man-in-the-middle attacks. That's what certification authorities are for. The client verifies that the server's certificate was signed by a CA. An attacker can either:

Leave the existing certificate, in which case he doesn't have the server's private key and cannot get access to the conversation, or
Replace the certificate, in which case it will fail verification (either it contains the real website's address but isn't signed by a CA, so the browser complains; or it contains an address that belongs to the attacker, so a CA would sign it, but it's not the address the client requested, so the browser complains)

2007-01-28 Reply Admin

Kiss me:
Someone buy this guy a book.

I hope this guy writes a book. I could use a good anti-pattern non-fiction novel. :)

2007-01-29 Reply Admin

What language is the OP coded in? C#? Can strings be compared with == there?

2007-01-29 Reply Admin

50% wrong! is not "select " but "select count()" (please, don't use databases like a rubbish bin...)

viraptor · 2007-01-29 Reply Admin

danixdefcon5:
I don't know how they implement "md5", but md5(md5('doofus')) will return 'doofus' in MySQL.

In short - stop spreading FUD! md5(md5('doofus')) = e03392cf95d418a9d25243894f54a889

2007-01-29 Reply Admin

Sha-Noo-Noo:
Kiss me:
Someone buy this guy a book.

I hope this guy writes a book. I could use a good anti-pattern non-fiction novel. :)

Nah. It would be as useless as the uncyclopedia. It's fun at first, but then you realize it's only piling up absurd statements and repeating cliches. Quite boring, like late Leslie Nielsen movies. They almost need canned laughs.

2007-01-29 Reply Admin

With reference to comments like this:

I sure hope you're kidding. You should never, ever, ever tell the user if the issue with their login was the password or the username, or both.

In the case of web based authentication, confirming to the user that their username is correct but the password they have entered is wrong is not in itself a security hole, but it is very convenient for end users.

If you enforce reasonably strong passwords, giving someone the username will make zero practical difference to their ability to brute force the login information.

It seems worth mentioning at this point you should check for failed login attempts - and act appropriately when brute force attacks are attempted from a specfic location and/or on a specific username.

Caveat: If the username is an email address and if your site is of a "senstive" nature (e.g. a porn site), then this approach would be ill advised.

It should be obvious when it's going to be suitable to do this and when it's not, but to suggest that it's 'never' appropriate and that displaying a user friendly message indicating that the user got their username correct, but the password was incorrect is a significant security risk is just plain wrong (FUD).

If your passwords are not strong enough to make knowing the username effectively a moot point, then you have a far more pressing problem you need to deal with.

2007-01-29 Reply Admin

cklam:
Aaron:
foofoodyne:
Killen:
Aaron:
The Real WTF™®? is that he didn't use a salt.

Actually, there's no proof that he didn't use a salt in this example.

Second that vote.
I'll grant you that it's not impossible, but I'll bet you dollars to donuts that the calling code looks like:
SomeObject.Authenticate(
 txtUserName.Text, 
 FormsAuthentication.HashPasswordForStoringInConfigFile(txtPassword.Text, "SHA1")
);
Most likely stuck directly in a "Click" event handler.

On an aside, I can't believe the number of people here who clearly have no understanding of what a salt is for. Generating a random hash during authentication does not help prevent a dictionary attack. That's just a half-baked implementation of a security context token, and it doesn't work at all when you have no control over the "client" (which you don't on the web).

Just hash once. Further hashing actually makes you less secure because every hash carries the probability of collisions, and you're increasing that probability with each successive hash. Adding a random salt doesn't help, because anyone who can see the hashes can also see the salt. If you're worried about traffic sniffing, they came up with a solution for that years ago, it's called SSL!
SSL: Second that.

Salt and then hash. That is enough. That is application security, which should be implemented.

If you worried about network security, then you use secure network mechanisms.

If you are worried about client (PC) security: Often, you do not have any control over this. If you have your user accessing your application from non-secured and/or compromised clients, then that's that - case closed: access to your application for the users in question is compromised.

That's bad enough - but the problem can be mitigated depending on the nature of the application: i. e. for e-banking web apps two-factor (or even three-factor authentication) should be implemented (at least) for performing actual transcations.

One way to do this are one-time-passwords (in Germany they are called TAN (short for german Transaktionsnummer - in english transaction number). You get a pad (called a block)in a secure tamperproof envelope sent to you by mail from your bank which contains about one hundred transactions numbers. Any new block (with any transaction number) contained therein) needs to activated by the end user in the banking app using an unused transaction number from the current TAN block. Doing this successfully deactivates the current TAN block and at the same time activates the new TAN block.

Before, you as an end user could select at your will which TAN from the current TAN block to use for a transaction. Nowadays (presumably because users were running out TAN on their current block or writing them down elesewhere/storeing them on PDAs or whatever) the banking apps requests you provide a TAN specified by row and cloumn on the original TAN block paper.

Of course, this is not foolproof: The biggest WTF that you can do with this scheme is to store your banking app login info together with the TAN block. That is why the banking apps in Germany do not require TANs for login authentication.

Though, those schemes are still vulnerable to MiM attacks. Something banks in sweden have noticed as of late. Nordea has had a few suck attacks directed at their clients the last few years. Most of them discovered due to crappy frontpages used by the attacker, but one kept going for a week or two I think, before someone computer savvy customer used it and saw what it was.

2007-01-29 Reply Admin

Kiss me:
I must admit I was looking for code like
if (struser == "testuser")
{
    this.authenticated = true;
}
But it's so rich! Here we go - misused SELECT, no WHERE clause, just plain wrong true/false setting. The only positive thing is that it is not SQL injection prone. But I think that's just an accident. Someone buy this guy a book.

You get points for being the first person to bring this up. Adding a where clause to this is NOT a solution, and is in fact another WTF because you are now about to assemble a SQL query from USER PROVIDED input.. That's just begging to be left open to an injection attack. All input is evil, and should be treated as such.

This should all get handled by a parameterized sproc, and there ought to be some input testing making sure that all chars in the username at least are 'allowed' chars..

FrostCat · 2007-01-29 Reply Admin

Come one, "developer tries to do a table scan instead of a unique select ... from ... where ..." has been done before.

FrostCat · 2007-01-29 Reply Admin

Come on, "developer tries to do a table scan instead of a unique select ... from ... where ..." has been done before.

cklam · 2007-01-29 Reply Admin

BruteForce:
cklam:
Aaron:
foofoodyne:
Killen:
Aaron:
The Real WTF™®? is that he didn't use a salt.

Actually, there's no proof that he didn't use a salt in this example.

Second that vote.
I'll grant you that it's not impossible, but I'll bet you dollars to donuts that the calling code looks like:
SomeObject.Authenticate(
 txtUserName.Text, 
 FormsAuthentication.HashPasswordForStoringInConfigFile(txtPassword.Text, "SHA1")
);
Most likely stuck directly in a "Click" event handler.

On an aside, I can't believe the number of people here who clearly have no understanding of what a salt is for. Generating a random hash during authentication does not help prevent a dictionary attack. That's just a half-baked implementation of a security context token, and it doesn't work at all when you have no control over the "client" (which you don't on the web).

Just hash once. Further hashing actually makes you less secure because every hash carries the probability of collisions, and you're increasing that probability with each successive hash. Adding a random salt doesn't help, because anyone who can see the hashes can also see the salt. If you're worried about traffic sniffing, they came up with a solution for that years ago, it's called SSL!
SSL: Second that.

Salt and then hash. That is enough. That is application security, which should be implemented.

If you worried about network security, then you use secure network mechanisms.

If you are worried about client (PC) security: Often, you do not have any control over this. If you have your user accessing your application from non-secured and/or compromised clients, then that's that - case closed: access to your application for the users in question is compromised.

That's bad enough - but the problem can be mitigated depending on the nature of the application: i. e. for e-banking web apps two-factor (or even three-factor authentication) should be implemented (at least) for performing actual transcations.

One way to do this are one-time-passwords (in Germany they are called TAN (short for german Transaktionsnummer - in english transaction number). You get a pad (called a block)in a secure tamperproof envelope sent to you by mail from your bank which contains about one hundred transactions numbers. Any new block (with any transaction number) contained therein) needs to activated by the end user in the banking app using an unused transaction number from the current TAN block. Doing this successfully deactivates the current TAN block and at the same time activates the new TAN block.

Before, you as an end user could select at your will which TAN from the current TAN block to use for a transaction. Nowadays (presumably because users were running out TAN on their current block or writing them down elesewhere/storeing them on PDAs or whatever) the banking apps requests you provide a TAN specified by row and cloumn on the original TAN block paper.

Of course, this is not foolproof: The biggest WTF that you can do with this scheme is to store your banking app login info together with the TAN block. That is why the banking apps in Germany do not require TANs for login authentication.
Though, those schemes are still vulnerable to MiM attacks. Something banks in sweden have noticed as of late. Nordea has had a few suck attacks directed at their clients the last few years. Most of them discovered due to crappy frontpages used by the attacker, but one kept going for a week or two I think, before someone computer savvy customer used it and saw what it was.

Sadly: yes - it is will not eliminate Man-In-The-Middle-Attacks. On top of the scheme described above SSL is always used, too (at least in Germany).

Security is not about being perfect - after all, what man can lock man can also unlock again ... this age-old adage has been valid throughout the ages and is even more valid nowadays. Security is about lowering the odds for comprimising the secured system/items/whatever for an attacker:

If an attacker has to go through too much trouble (i. e. use too much time/ paying too much money etc.) negating the benefit he will gain by compromising the targeted system/items then he will abstain from compromising the targeted system/items.

The schemes described above are in full production use in the banking industry in Germany (and problably a number of other european countries, as well - although I can not comfirm that). Together with SSL, IMHO they raise the bar significantly compared to a simple non-SSL username/password as it is employed with some US banks (I recall some news items during the last month on network-world.com on this subject).

Back to the scheme described above: I omitted that you never have to validate your login with a TAN. And the banking get extensively briefed about that (and about e-mails purporting to be from your bank) ... still there are some people who willingly click on a link in a phishing e-mail and login on a phishing web site with user/account-id, password, additional login info as required by the real banking site and a TAN ... in my opinion: this is 2007 and how stupid can you be not to have heard the shot(s) by now ... just proves to me that there always will be fools and the people who will take advantage of them.

Philosophical: I wonder if this is basic human nature, or what !?!

chrismcb · 2007-01-30 Reply Admin

jpa:
I'm surprised that no-one has yet to mention the insecurity of having a testuser stored in the database, and probably also the password for it in the unit testing code.

I suppose you only run your tests against a live data base using real customer data?

2007-01-30 Reply Admin

The code doesn't even validate under unit testing except in the single case that the username/password being tested was the last record returned in the data set. The repetition structure sets this.authenticated after each iteration based on the value returned in that iteration. So the first incorrect match after the correct match would reset this.authenticated to false even though the function should have resulted in an assignment of truth. Of course proper use of the WHERE clause would make the whole thing much more efficient.

The original code: public void Authenticate( string username, string passhash ) { SqlDataReader source = _Database.Query("SELECT * FROM users;");

  while ( source.Read() )
  {
      if ( source["user"].ToString() == username
      &&   source["pass"].ToString() == passhash )
      {
          this.authenticated = true;
      }
      else
      {
          this.authenticated = false;
      }
     
  }

}

A better implementation: public void Authenticate( string username, string passhash ) { SqlDataReader source = _Database.Query("SELECT * FROM users;"); this.authenticated = false; while ( source.Read() ) { if ( source["user"].ToString() == username && source["pass"].ToString() == passhash ) { this.authenticated = true; break; } } }

2007-01-30 Reply Admin

Xaroth:
Here's a fun trick for existence checking that doesn't require the database to loop over every matching row to count them up, when you really only care if it's there or not:
SELECT 1 FROM whatever WHERE your_criteria=something LIMIT 1;

Nice. The Oracle equivalent is:

SELECT 1 FROM whatever WHERE your_criteria=something AND ROWNUM = 1;

But It Worked in the Demo

Leave a comment on “But It Worked in the Demo”