- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Okay, you are both wrong.
Anon: There is NO search on the text. There is just an insert into a db table with a unique contraint on the password field. (Though it is true that the value entered into that field could be either the plain-text password or the hashed password. Either way, it would not affect the WTF).
Kip: Your collision probablity is only true if every user magically chooses a unique password. (Unless you're saying that a new random salt is generated for each password, but I think even in this system, they may have caught that, as it would mean that no one would ever be able to log-in.
Admin
First fist?
F A S T
Admin
Right to the point. We don't know whether this constraint is actually doing something useful and sensible as ptomblin says.
The REAL wtf is that, if the DBA changes the constraint name, then the check for duplicate passwords will fail SILENTLY, with no complaint at either compile time or run time, which means that the error will go undetected unless:
Admin
I suspect not.
Admin
There's another problem here. We had some INSERT code which relied on a "violation of Unique Key" error message to detect an existing row in the table. If the attempted INSERT got the error, then the code did an UPDATE.
This worked well enough when unit testing when the table had something like 5 rows. But when the table grew to just about 100 rows, the server would crash.
This reminds me of an earlier WTF discussion on whether to use if-then or rely on exceptions. The word from one of the Java gurus (someone who's worked a lot on JVM development) is that, even in a good scenario, an exception has about 10x more overhead than an if-then.
Admin
lol... sounds like the application was so horrible, they spent more time coding error handling routines than fixing the bugs they exposed. WTF man. Using strings for what should be an error number? WTF man... It's obvious that string is constructed too. Why couldn't they use an error numbering system of some kind? Even if you want really descriptive errors, that can be done with bitfields and such. WTF man.
OH WHAT THE ($#. I thought the code was bad enough, but the LOGIC?!?!?!?!?!? WTF MAN!!!!!!!! DON'T TELL PEOPLE THAT A PASSWORD IS ALREADY IN USE DUMB#(&!
Admin
Ok I don't think anyone has pointed this out yet.
Half the hack is getting the password, the other half is getting the user name.
So basically you could start registering usernames, and record one every time an error message returns "sorry, that name is taken"
Do the same for passwords.
After a while you've got a pretty good list of usernames and passwords, now you've just got to try every password on each username. Narrows it down a bit.
Admin
lol, yeah, i did the same thing lol...
Admin
I bet this is the table structure
tblUser - userid, user
tblPassword - passwordid, password, iscurrent
tblUserPassword - userid, passwordid
Maybe it makes them feel better to have the passwords in a seperate 'secure' table
Admin
OMFG... this one made melaugh out loud and everyone is LOOKING AT ME![:#]
Admin
They could improve "userfriendlieness" by suggesting some new password options, i.e.
"The password `secret' is already in use... Other suggested passwords:
X secret_2006
_secret_
1337_secret
secret_<your_login_name>
Enter your own: ____
"
(Obviously, the next error message would most likely be "You password may not contain underscores"...)
Admin
For this system, how bad could the effect of this particular piece of code possibly be? I'd be willing to lay pretty good odds down on being able to access "secured" resources on this system without providing any kind of credentials. So tipping your hand revealing the passwords in the system becomes a moot point.
Admin
Isn't that the purpose of salt? Hypothetically, say we both choose the password "12345", my salt is "kipt" and yours is "jame". My hash would look something like "kipt_" . md5("kipt12345"), and yours would be something like "jame_" . md5("jame12345"). We'd both still be able to log in. And the salt doesn't have to be appended to the hash, it could be stored in a separate column, or you could make the salt something slightly more creative (like a hash of the username). The salt doesn't have to be protected information. Knowing the salt would only make a dictionary attack easier. But we're not talking about a dictionary attack here.
Of course, I've never designed any system like this, so I don't know how secure the particular methods I mentioned would be. But I think that's basically how salt is used, from what I remember from my Secure Programming course in university (3-4 years ago).
So yeah my theoretical probability is probably a little low if lots of users want to use "Miss Kitty" as their password or something. But if the salt is sufficiently long and random it would still take billions of users before this error were encountered.
Admin
Could anyone explain me what is the difference between following attacks:
1. Try to create new accounts, and check for an error.
2. Try to log on an account using different passwords?
I think I know the answer: the first one is easier to spot by administrator. But I do agree that it is a little bit faster -> you'll get a password 1000 times faster if there are 1000 users....is it really a WTF?
Admin
Did anyone else notice too, how it's using the string.IndexOf method rather then a simple comparison?
I mean it works...but it's odd....
Admin
like, lol! i mean omg, lol?
lol.
Admin
That's nice :)
And what's better is than it's tested on english version of MSSQL only... german version may return a different error message. Just plain brilliant! And it's not just checking whether the password already exists or so but attempting to update and check how it failed. Oh well...
Admin
You didn't happen to work for a large online book retailer, did you?
Admin
Ooh, that would be quite awful. You may be correct, too. It's tough to guess how the tables are laid out.
I was pointing out that passwords were probably not used as a key, as someone else had feared. It was given a unique key constraint, but that doesn't mean password was used as a key, which would be a bigger WTF than the code. It looked to me like it is just how they enforce the silly requirement that no two users have the same password.
Admin
I think you are assuming that you don't know any of the usernames. An application that has a unique constraint on the password field would have to be a very small app used by a small group of people. There's no way that eBay would accidentally have this "feature" slipped in.
If we are talking about a small group of people, it is most likely an internal app used my a small office or small company. Once you figure out that a password is taken, all you have to do is try to log in as each person on your company phone list and you're done.
--
CAPTCHA: why doesn't "reference" ever work?
Admin
I assume the code is Java. The indexOf method will return the location of the parameter String or -1 if it is not found. This is different from a comparison in that the Strings don't have to be the same, just the errMsg must contain that String in it somewhere. Otherwise you would have to compare against the entire errorMsg wich would be a much longer String.
Admin
I bet that he is receiving an array of exceptions, then he concatenates them all together, and then searchs using IndexOf.
This probably means that all exceptions get discarded except that one. He probably has something like:
And looking for the error number will be probably useless in this case, since he would probably look for code 34 "Unique key constraint", and wind up confusing it with error 1344 "Too many connections for this hardware, buy a new server" because of using IndexOf on a concatenated string. (I made up the codes).
(I know about this last problem because I tried something similar time ago. Oh, the joys of chasing during hours that kind of misleading bugs.)
Admin
Which, of course, begs the question: Why didn't he use a smaller string? He could have simply searched for the name of the violated constraint. At least that way, it would be relatively stable across db languages, and would at least be more akin to looking for a "return code" of sorts.
Since the return code from this function is a string, I think it's safe to assume that he thinks this is an acceptable method of passing errors around.
Admin
12345? Thats the same combination for my luggage! Remind me to change my luggage combination...
Admin
I was actually asking myself the other day whether there ever has been someone stupid enough to put a unique constraint on a password field.
Thank you for taking the last bit of faith I had in humanity.
Admin
I keep seeing people suggest this. But it's not necessarily true: this could be a signfigant issue.
Think - you're a hacker, and trying to come up with a username/password pair. You usually start with a known username and try to get the password for it. But if you're starting with a password you know to be good, all you need to do is find out maybe one or two other people's usernames, then sit down with a copy of the corporate directory to work out the rest.
So yeah, I'm thinking that could be a very big deal.
Admin
How much do you want to bet that errMsg is really just the entire page's buffered HTML?
Admin
I guess since this a "behemoth" app, it was build for a rather large company.
As we all know large companies tend to standardize logins ... .
so you don't even have to check for existing usernames
Creating a new user in the system
Admin
Of course a new random salt would be generated for every user's password. If not, you wouldn't use salt in the first place. This is exactly how UNIX does things, and oddly enough people are able to log in to UNIX systems all the time. The reason you can log in as that during the crypt-and-compare process you can tell, by looking at the stored (crypted) password, what salt you'll need to use. (In fact, you normally just pass the crypted password as the salt when checking for a password match.)
Now, UNIX does this for security reasons. If I happen to pick the same password as Joe Hacker, I'd rather he not be able to tell that just be looking at the password file. Plus if there are enough salt variations, then it becomes impractical to store a pre-crypt()ed dictionary for brute-force attacks.
Using salt to avoid a hard constraint on matching password representations in the database seems like a bit of a hack, because now when the user picks a password that's already in use, it's a random shot whether or not it'll be accepted. At a glance, it's better if the program responds by silently picking a different salt value and trying again.
Of course, if one particular password is really, really popular, then you'd need some way to limit retries. (Maybe pick the salt randomly, but then cycle through salt values sequentially during retries?) But then if you do find that you just can't use the password (all salts are taken), you're giving the user a lot of information ("Everyone and his grandmother is using the password you've selected. Hack away.") Much better if you have enough salt that this never happens.
So, like I said, it's a hack. Viability depends on size of userbase and prone-ness of users to all select the same password; but in any case a better approach is to protect the password file and not worry about dup passwords. (At which point you probably don't need salt as far as I can tell...)
Admin
Depending on implementation, I don't see a problem with "update on failed insert" as an upsert strategy. TeraData does it the other way around, but either way should be viable.
For some DBMS's, there may not be an efficient way to do this -- but if 100 rows overloaded the system to the point it was crashing, then the database software is the WTF.
Admin
return "The password entered is already in use by " + username + ". Why don't you login as him/her and change it so you can use it?";
--Rank
Admin
Ha! That's the beauty of it! They must be, or else why would it be violating a unique key constraint? Just the idea of a variable, user entered, key is enough to make any database jockey worth a dollar cringe. just a terrible, terrible idea...And what the hell happens to the data if the password changes? Unless its a huge flat table, they're having to run cascading updates ON KEYS! That's some scary stuff.
Admin
I'm imagining that you could also pull false positives by having a random salt as well, couldn't you? I mean, just because
doesn't mean
does it? Even if the chances are pretty low...
CAPTCHA = dinky
Foreshadowing, your sign of a quality CAPTCHA
Admin
That adds a new meaning to "stealing a password".
Sincerely,
Gene WirchenkoAdmin
that's why i suggested a bitfield. bitfields rule!
Admin
Oh, and I was the one doing the "fist!" first reply today. I just saw "0 replies" and I had to do it....
No, actually, you did not have to.
Admin
It's seems most wireless routers ship with the IP address of 192.168.2.1 or 192.168.1.1, and with a username admin and password admin or blank.
I'm wondering how fun it would be to code a little Windows service on my laptop and drive around town....
There goes all of my Karma from returning that wallet with the money still in it.
Admin
why not just do away with passwords altogether. Far too much effort.
Also provide the user a list of valid user names and allow them to choose one or create a new one. User names that can transfer over 6 figures in US dollars to an account of your choosing should be marked with a special character in order to make the application user friendly.
Admin
Here's a theory ... Some PHB found out someone's password was a swear word, and we all know how evil those are, so he said to the intern "I want to keep people from using swear words as their passwords and I want to be able to add to the list". So the intern adds these two lines of code to the ongoing mess and starts adding user/pwds: (userf*$#, f*$#), (userd*$#, d*$#), (userphb, the boss is a schmuck).
Admin
Actually it should, if the salt is done right. Again consider the UNIX implementation. Now, in UNIX crypt(), the salt is actually a second parameter to crypt, as in crypt(pw, salt). The first characters of the result (returned by crypt()) will be the salt, which is of fixed length (on any given UNIX implementaton). So the only way for two encrypted password strings to match is for the salt to match (because otherwise those first few bytes would differ); and if te salt matches and the rest of the encrypted string also matches, then the passwords must have been the same.
If you have a salt scheme for which this property does not hold, then what you have isn't useful salt for password encryption. Remember, a false positive would mean someone can log in without actually havnig your correct password, essentially eliminating at least half of the keyspace.
Admin
User1: How can i remember the password when all the good ones are used?
User2: You should have been here at the start, i managed to get "password".
User1: So you're the bastard that stole that!
Admin
Drum D. -
Spellcheck your sig ...
"Two things are infinite: the universe and human supidity, even though I'm not yet sure about the universe.
- A. Einstein "
Admin
Actually, he or she did have to make that post or risk banishment. You see, Alex checks the IP of the first user who reads the day's WTF, and if that person does not make a "first" or "fist" post, he bans that IP for a month.
Unless someone else has a better explanation for the plethora of fist posts, I'm sticking with that one.
Admin
I'm getting this horrible vision that explains why the password field has a uniqueness constraint: there is no username. You are identified AND authenticated with just the password. The code we are seeing is part of changing your password once you are logged on.
It is a braindead and absurd idea yet it's natural simplicity suggests it is the most likely reason why there is something so braindead and absurd. Must drill hole in head to let out madness.
Admin
Password and no ID is a paradigm used in some phone systems for calling long distance. It is still braindead and absurd.
Please do not do that. With the horrible code that you get exposed to around here, drilling a hole will let the madness in. Or more of it, anyway.
Sincerely,
Gene Wirchenko
Admin
But he IS right. No one would ever guess it. How can you guess something when you already know it?
Admin
I agree with you. If running an "update if failed insert" on 100 rows overloaded their system, it's time to upgrade the 8086 and tape cassette to something a little more modern. Perhaps a calculator?
Admin
What's wrong with forbidding different users from having the same password? I know of at least one system where no password may ever be even reused, whether it was you or another user that formerly employed it.
Admin
Ok, I'll cave first.
Please, please, please let us see more lines of this code... there must be some other (even if not quite so WTFy) lines in there.
Do they store users SSN in a StorrayEngine? or maybe have a
passwordInUse( pass ){
if( isNullOrMaybe( pass ) ){
//...
} else if( isFileNotFound( pass ) ) {
//...
}
}
Captcha complaint of the day: Is "tastey" a word?
Guess it goes well with "salad"
Admin
What assurance do you have that the error number will be any more stable than the error message? Both can break very easily.