Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »

het: Nov 14, 2002; A dark black past
is my most valued
possession

Mogomra posted:

One of my previous employers had the same kind of thing going on. Passwords were of course stored in plain text in the database, and whenever a new customer signed up, they got the same default password that everyone else got. They were never really prompted to change that anywhere, except maybe a "don't forget to change that password!" in the email that sent them it.

Out of curiosity, did you ever check to see how many accounts still had the default password?

# ? Feb 27, 2013 19:05

Adbot: ADBOT LOVES YOU

# ? May 17, 2024 14:28

bucketmouse: Aug 16, 2004; we con-trol the ho-ri-zon-tal
we con-trol the verrr-ti-cal

Do any of you have to work with people who have bizarre coding standards or name their temporaries weird things for no good reason? There's a particular person whose contributions can be easily detected by hundreds of lines of unnecessary comments and temporaries named 'looky'. Also they hate multiline comments for some reason!

code:


int looky = 5; // Assign value 5 to looky.
  	       // Will be overwritten in loop.

Codebases without style standards depress me.

# ? Feb 27, 2013 19:07

Rottbott: Jul 27, 2006; DMC

Yes - one person who insisted on writing a C-style comment for pretty much every line of code, which drove me mad for obvious reasons:

code:

/* Is a true? */
if (a)
{
	/* Yes */
	/* Set b to 5 */
	b = 5;
}
else
{
	/* No */
	/* Do a thing */
	DoAThing();
}

# ? Feb 27, 2013 19:12

zergstain: Dec 15, 2005

armorer posted:

So the senior dev blamed his lovely code on the users being stupid? That is one of the most ridiculous sidesteps I've ever heard.

It could've been out of his hands. It was a few years ago.

# ? Feb 27, 2013 19:34

Mogomra: Nov 5, 2005; simply having a wonderful time

het posted:

Out of curiosity, did you ever check to see how many accounts still had the default password?

Oh yeah, can't believe I completely forgot the most important detail! There were somewhere between 50 and 100 clients out of maybe ~1.5k that actually changed their passwords. Ever.

# ? Feb 27, 2013 19:49

Gazpacho: Jun 18, 2004; by Fluffdaddy; Slippery Tilde

armorer posted:

So the senior dev blamed his lovely code on the users being stupid? That is one of the most ridiculous sidesteps I've ever heard.

In that context he's probably right.

# ? Feb 27, 2013 22:00

zergstain: Dec 15, 2005

I was thinking that, but then, if Facebook users can handle a password reset (I'm assuming they don't email your password if you forgot it, never had to use that feature), then any site's users could. Also, he might not have been the one responsible for the users table.

# ? Feb 28, 2013 00:39

how!!: Nov 19, 2011; by angerbot

I worked for a company once that had an e-book they were selling on how to be successful. It cost like $500 for access to the book. The code that handled the login just redirected them to the book regardless of what you entered into the username/password field. You could literally type in gibberish and it would let you in. They had it done this way because the users were too stupid to realize they could get in for free, and didn't want to have to deal with lost/forgotten passwords and crap like that.

# ? Feb 28, 2013 01:16

Sang-: Nov 2, 2007

How long did the rewrite take?

# ? Feb 28, 2013 01:29

bobthecheese: Jun 7, 2006; Although I've never met Martha Stewart, I'll probably never birth her child.

php:

<?
Order\Get_Info->clientIsAbleToShipResmed($orderID, $database_link);
?>

There is nothing not wrong with this.

Order is the namespace, which would be ok if it held the order related code and nothing else, but, as is clear, it doesn't.

Get_Info is a class name. There is a class called "Get_Info". Seriously.

clientIsAbleToShipResmed is at least descriptive. I'll give it that. You know what you expect it to do (kind of). Still horribly named, though.

$orderID... so we're passing an order ID to find the client to check if they're able to ship a particular type of product? Why not pass the client ID? What happens if the client doesn't have an order yet? I guess that passing the orderID might be justification for having it in the Order namespace...

$database_link is... we're passing a link to the database around to functions. what. the. gently caress.

Then the whole thing is a clusterfuck of naming conventions.

Sometimes I really want to just start beating people until they stop writing code.

bobthecheese fucked around with this message at 03:08 on Feb 28, 2013

# ? Feb 28, 2013 03:03

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

You're lucky. You could be much worse off.

# ? Feb 28, 2013 04:10

bobthecheese: Jun 7, 2006; Although I've never met Martha Stewart, I'll probably never birth her child.

Suspicious Dish posted:

You're lucky. You could be much worse off.

This is a new refactor. This is the "improvement" on what was already there.

# ? Feb 28, 2013 05:06

Tei: Feb 19, 2011

The migration from "storing passwords" to "storing hashes" is quick and painless.

1) you create a column hash_password
2) a update fill that colum with your hashed value. maybe to this hash_password = md5(concat ( id_client, password, "_SALT_"))
3) update the auth code to check both columns WHERE (password = ? or hash_password = ?)
4) nuke the original password colum.

from this point, the code will use the hashed version.

more changes are needed:

5) Update the "lost password" so it gives a link to reset the password to the original registered email
6) Update the registering/changing password procedures, so only hash_password is filled.

optional, recomended

7) Remove any reference to "password" in the code, and the database.

It just something that cost time,and give no immediate profit, so I imagine is hard to sell it. Other than "if our database is access by crackers, nobody will have the passwords, and we will be able to say that in public".

edit:
ooops.. forgot the actual hash function

Tei fucked around with this message at 22:28 on Feb 28, 2013

# ? Feb 28, 2013 12:48

nielsm: Jun 1, 2009

Congratulations, you just described a broken password hashing system.

# ? Feb 28, 2013 13:04

pigdog: Apr 23, 2004; by Smythe

To clarify, you'd want to use a different, randomly generated salt (stored nearby) for every hash. (And to actually use a hash function rather than concat(), though I suppose that's just a posting typo)

pigdog fucked around with this message at 13:16 on Feb 28, 2013

# ? Feb 28, 2013 13:13

God of Mischief: Oct 22, 2010

The (commercial, very expensive) e-commerce framework we have at work can't support anything more advanced than MD5. That is close enough to plain-text passwords to not even be worth it. :mad:

# ? Feb 28, 2013 13:15

Zombywuf: Mar 29, 2008

You missed "take a backup and stick it on an external disconnected device".

# ? Feb 28, 2013 15:02

MononcQc: May 29, 2007

My first job had passwords hashed, but it was MD5 with no salt. You could just pick the most popular password from the DB, brute-force it in seconds, and have access to thousands of accounts.

I voiced my concerns but nobody cared

# ? Feb 28, 2013 15:49

Progressive JPEG: Feb 19, 2003

MononcQc posted:

My first job had passwords hashed, but it was MD5 with no salt. You could just pick the most popular password from the DB, brute-force it in seconds, and have access to thousands of accounts.

I voiced my concerns but nobody cared

I once worked on a project where the passwords were sent in unencrypted cleartext, then MD5'ed without salt for storage in a DB. Oh and the login itself was pretty much honor system in the first place; there were no server-side constraints, the login response just allowed clients to decide what things to obscure. Anyone with knowledge of wireshark could do pretty much whatever they wanted.

Even though the thing was only used in practice within an intranet, I eventually decided I'd better cover my own rear end by putting things in writing in the form of a long and descriptive bug report, which was deferred.

# ? Feb 28, 2013 17:08

MononcQc: May 29, 2007

The best one we had at that same place was management decided that members could sign in to the site from the e-mails we sent them every week with new dating option (it was a dating site, I wrote the system that generated match-making mails).

Developers argued a lot against auto-logins: members with an active session would be fine, and those with old sessions or accounts would need to log back in. Allowing to log-in from e-mails we sent meant we needed an explicit session ID stored in URLs sent to members, and copy/pasting email links was risky on top of providing wide-open session hijacking options.

They decided to go through with it anyway. At some point, whoever was integrating the templates for the e-mail (it might have been me, even) made a mistake in how PHP handled references in foreach loops (see http://php.net/manual/en/control-structures.foreach.php) or something similar, and we sent a batch of 80,000 e-mails with mixed up session IDs, where for a few days, people could log in using accounts that weren't theirs.

So of course this created a poo poo storm, so we decided to cancel all sessions and force users to reconnect, which yielded a poo poo ton of complaints about users who somehow managed to keep their session active for 2+ years and suddenly had lost their beloved session, couldn't remember their passwords, and had no idea what e-mail account reseted passwords went to because it had been so long ago. Then it became a job of updating their records to their new e-mail addresses after customer service managed to confirm their identities, and so on.

Everything that could have went wrong, went wrong. I learned a lot about what not to do as a developer at that place :unsmith:

# ? Feb 28, 2013 18:53

ExcessBLarg!: Sep 1, 2001

nielsm posted:

Congratulations, you just described a broken password hashing system.

pigdog posted:

To clarify, you'd want to use a different, randomly generated salt (stored nearby) for every hash. (And to actually use a hash function rather than concat(),

Before you get another "Congrats!" response, it would be nice if someone explains why it's broken.

"hash(password + salt)" was pretty good in the 90s. However passwords themselves don't contain much entropy and these days hardware can be cheaply acquired to crack passwords "relatively" quickly.

The answer to that is to use, in place of a standard cryptographic hash function, a password-based key derivation function, which have adjustable work factors so as to scale up with CPU performance increases. PBKDF2 and bcrypt are two such, except that both fold under massively-parallel cracking hardware, which is addressed by scrypt and it's adjustable memory factor.

But really, the ultimate answer is that you shouldn't ever roll your own crypto, but use a library that does it right, and can update when what was previously "right" is now considered "horribly wrong only an idiot would do that!"

# ? Feb 28, 2013 19:24

substitute: Aug 30, 2003; you for my mum

Where/what is the best way to store the randomly generated salt, to use for verification/login later?

# ? Feb 28, 2013 22:59

shrughes: Oct 11, 2008; (call/cc call/cc)

substitute posted:

Where/what is the best way to store the randomly generated salt, to use for verification/login later?

In the database right next to the password.

Edit: Let me be clear, this is not a joke, it's not some cynical ironic answer, it's actually the correct answer.

Note that you shouldn't even have a separate salt -- you're asking a wrong question. The library you use should be creating the salt for you. For example, a bcrypt password digest library will take (password, work_factor) as input, and then generate a salt itself and output an opaque string that contains both the salt it generated and your slowly hashed password. Then some verify_password function takes (opaque_string, password) as input and verifies the password. You never should have to use the word "salt" yourself in any mainstream programming language.

shrughes fucked around with this message at 23:05 on Feb 28, 2013

# ? Feb 28, 2013 23:03

Zamujasa: Oct 27, 2010; Bread Liar

shrughes posted:

In the database right next to the password.

Edit: Let me be clear, this is not a joke, it's not some cynical ironic answer, it's actually the correct answer.

Note that you shouldn't even have a separate salt -- you're asking a wrong question.

So technically you shouldn't store it next to the password at all :pseudo:

I like the idea of scrypt, though, but I think for the time being our project is good with bcrypt.

When it comes to password encryption, never ever ever roll your own, ever. Just don't.

# ? Feb 28, 2013 23:45

Wheany: Mar 17, 2006; Spinya^{ha^{ha^haha}ha}ha_{ha_{ha_haha}ha}ha!; Doctor Rope

Yeah, the password-specific salt isn't secret.

I think the point is that if you had a global salt and you saw that several users have the same hash, you would only need to brote force the one hash and you would know the password of any user with the same hash.

You could probably even do things like sort the hashes by frequency and then the most frequent hash would correspond to "password" or "123456" or whatever is the most common lovely password.

# ? Mar 1, 2013 00:01

gnatalie: Jul 1, 2003; blasting women into space

I'm currently completely re-writing an application with 90k lines of code and there's not a single function that has a return. Need to calculate something or append a string? Pass it by reference! And there's around 20,000 if-then statements. It was originally written in VB6, I'm using .Net 4.5 + WPF, but still :cry:

# ? Mar 1, 2013 00:10

Pilsner: Nov 23, 2002

candy for breakfast posted:

I'm currently completely re-writing an application with 90k lines of code and there's not a single function that has a return. Need to calculate something or append a string? Pass it by reference! And there's around 20,000 if-then statements. It was originally written in VB6, I'm using .Net 4.5 + WPF, but still

It could be worse, if it isn't already: All variables that need to be passed around are private variables at the root of the class, and methods in the class just manipulate said variables as necessary. 100% impossible to follow the flow of the program or unit test it. :argh:

# ? Mar 1, 2013 01:13

New Yorp New Yorp: Jul 18, 2003; Only in Kenya.; Pillbug

Pilsner posted:

It could be worse, if it isn't already: All variables that need to be passed around are private variables at the root of the class, and methods in the class just manipulate said variables as necessary. 100% impossible to follow the flow of the program or unit test it.

It could be worse: They could be public fields. :v:

# ? Mar 1, 2013 01:21

ExcessBLarg!: Sep 1, 2001

Wheany posted:

I think the point is that if you had a global salt and you saw that several users have the same hash, you would only need to brote force the one hash and you would know the password of any user with the same hash.

It's actually worse than that. With a global salt, you can do (e.g., dictionary) attacks against all users at once. Just check "hash(dictionary_word + salt)" against all the stored hashes.

Furthermore, only one user needs a weak password for you to get in. With per-password salts you don't necessarily know which passwords are weak, so you either have to guess which users might have weak passwords, or otherwise iterate through the entire list of users, wasting time on the stronger ones.

# ? Mar 1, 2013 02:55

Catalyst-proof: May 11, 2011; better waste some time with you

ExcessBLarg! posted:

It's actually worse than that. With a global salt, you can do (e.g., dictionary) attacks against all users at once. Just check "hash(dictionary_word + salt)" against all the stored hashes.

Furthermore, only one user needs a weak password for you to get in. With per-password salts you don't necessarily know which passwords are weak, so you either have to guess which users might have weak passwords, or otherwise iterate through the entire list of users, wasting time on the stronger ones.

Whoa! I never even thought about it like that. gently caress, I need to learn more about security and cryptography.

# ? Mar 1, 2013 03:00

bobthecheese: Jun 7, 2006; Although I've never met Martha Stewart, I'll probably never birth her child.

Wheany posted:

Yeah, the password-specific salt isn't secret.

I think the point is that if you had a global salt and you saw that several users have the same hash, you would only need to brote force the one hash and you would know the password of any user with the same hash.

You could probably even do things like sort the hashes by frequency and then the most frequent hash would correspond to "password" or "123456" or whatever is the most common lovely password.

I've seen, a number of times, a password being salted with not only a system-wide salt, but also the username. This adds the security of a good salt, with the "the same password is hashed differently for different users" thing.

I've never been shown evidence as to why this is a bad thing.

# ? Mar 1, 2013 04:18

DaTroof: Nov 16, 2000; CC LIMERICK CONTEST GRAND CHAMPION
There once was a poster named Troof
Who was getting quite long in the toof

bobthecheese posted:

I've seen, a number of times, a password being salted with not only a system-wide salt, but also the username. This adds the security of a good salt, with the "the same password is hashed differently for different users" thing.

I've never been shown evidence as to why this is a bad thing.

it's better than a system-wide salt by itself, but it adds the stipulation that changing the username invalidates the password. it still makes more sense to give every account its own salt and leave the other fields out of the equation.

# ? Mar 1, 2013 04:26

Carthag Tuek: Oct 15, 2005; Tider skal komme,
tider skal henrulle,
sl�gt skal f�lge sl�gters gang

DaTroof posted:

it's better than a system-wide salt by itself, but it adds the stipulation that changing the username invalidates the password. it still makes more sense to give every account its own salt and leave the other fields out of the equation.

Don't most sites require a password to change the username anyway (if changing it is even possible)?

# ? Mar 1, 2013 04:31

bobthecheese: Jun 7, 2006; Although I've never met Martha Stewart, I'll probably never birth her child.

Carthag posted:

Don't most sites require a password to change the username anyway (if changing it is even possible)?

Bingo. If a username is going to get changed, a password is usually required (at least it is in all situations where I've written a system that allows a username to change, which in itself is rare).

# ? Mar 1, 2013 04:34

shrughes: Oct 11, 2008; (call/cc call/cc)

bobthecheese posted:

I've seen, a number of times, a password being salted with not only a system-wide salt, but also the username. This adds the security of a good salt, with the "the same password is hashed differently for different users" thing.

I've never been shown evidence as to why this is a bad thing.

The same reason that using a per-user random salt (with a fast hash function) is a bad thing.

# ? Mar 1, 2013 04:40

how!!: Nov 19, 2011; by angerbot

bobthecheese posted:

Bingo. If a username is going to get changed, a password is usually required (at least it is in all situations where I've written a system that allows a username to change, which in itself is rare).

just use the freaking userid

# ? Mar 1, 2013 05:05

Deus Rex: Mar 5, 2005

how!! posted:

just use scrypt

# ? Mar 1, 2013 05:08

DaTroof: Nov 16, 2000; CC LIMERICK CONTEST GRAND CHAMPION
There once was a poster named Troof
Who was getting quite long in the toof

Carthag posted:

Don't most sites require a password to change the username anyway (if changing it is even possible)?

Good point. I'd still prefer to keep the salt and the hash discrete, but from a security standpoint, one is probably as good as the other.

# ? Mar 1, 2013 05:38

Comrade Gritty: Sep 19, 2011; This Machine Kills Fascists

Passlib is excellent for password storage in Python FWIW

# ? Mar 1, 2013 05:56

Adbot: ADBOT LOVES YOU

# ? May 17, 2024 14:28

shrughes: Oct 11, 2008; (call/cc call/cc)

DaTroof posted:

Good point. I'd still prefer to keep the salt and the hash discrete

What?

# ? Mar 1, 2013 06:13

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »