Today I audited some of the worst professional code I've ever seen. The code itself maybe not so bad other than a lack of any organizational structure and twenty layers deep of JavaScript callback hell.

But wow, what a complete lack of understanding of basic security. We all know to hash passwords now, right? Back in 1999 md5() was pretty cool. I'll admit. That's what this site started with. But today, it's not so great.

So these guys used md5 for their hash. We'll call the field password_md5. And it was right next to another plain text field in the same table called password. You have missed the point. Entirely.

Every single SQL query suffered from SQL injection attacks.

The SQL query to authenticate used "LIKE " + email... so you could enter '%' as your email to match any account.

But the worst? There is a public-accessible API url (because none of the endpoints require authentication) that literally is the language equivalent of print("SELECT * FROM user").

Have I described some code you recently wrote? Ouch. Get a new job.

The worst code I remember seeing right now is the gnugo code (the Go game). A total lack of control in the monitoring, too many functions accumulated that do the same and duplicate variables because nobody knew what the previous ones were doing.

My code is perfect.

Because it's all been rewritten 10 times.

@Edgar Reynaldo :
Curious, I have never needed to rewrite. I am so perfect that I have already forgotten how to be wrong.

I enjoyed this .

Nah, it's hashed bro

Wow.

Some weeks ago I had to refactor manual XML traversing code that would randomly crash. Turned out it exclusively matched attributes to identify elements, ignoring tags. Besides, there was a preference for matching attribute values without matching their names. Also, things like getChild(3) were used extensively. It went down a tree like this. On the way, it used some obscure helper functions to match elements that did entirely different things if empty string was passed as the fourth argument and so on. Of course, all constants were "inlined" with no regards to eliminating duplication. And yes, sometimes, the same constant was spelled a bit differently.

A buggy hand-written linked-list implementation, in C++. Lists of 3+ elements would be corrupted on erasing one element.

Separately, a few thousand lines of code, all with single-letter variable names, no comments.

Not professional, but you should see some of my early code.

I was very fond of single letter variables. Plus, Pascal is case independent so everything is capitalized.

My code's quite good because I avoid the need for header guards. I expect someone here'll tell me I'm silly to do so, but it seems to me since I learnt to program, programming in BASIC on the ZXSpectrum 48K, that header guards are there to facilitate a compromisation of the need to know what's going on in multiple source file based programs.

So you use pragma once or all your code is in 1 file ?

Or ForwardDeclareParty ?

No, I have lots of C source files and lots of headers. I just make sure that every header is included in the right order so that the headers know what they need to know from headers previously included. Does that sound crap? I'm willing to concede that it might be making life unnecessarily difficult.

I think that means I forward declare everything. Don't know what ForwardDeclareParty is.

Definitely not the worst that's out there, but my codebase at work is ~20k lines in embedded C, everything's in one file, ~300 functions that all return/take void, and the biggest functions are 1k lines long, ~200 global vars, nested switches, and if/elses nested 7 levels deep.

EDIT:
I almost never rant, so this is a rare occasion.

There is so much dupe code in our base, it's not funny. Often, people enable or give excuse for low-level languages to yield poor spaghetti code, but there really is no excuse. Unless you are developing for a device that either doesn't have a processor with a call stack, or at the very least, a jump instruction, there is absolutely no reason for you to duplicate code...ever, and no, loop unrolling to get more cycles is not a reason in this day and age.

Yes, I meant that you must have to do a lot of forward declaring.

I don't think that's bad, but it also does not guarantee good code; your code could still be full of buffer overflows

I've consumed a lot of alcohol over the years to forget, but I've encountered some terrible code in my travels.

When I first started working in the industry I was working on a VBScript-based ASP Web application that had zero code reuse, duplicating everything as needed (even within the same page/program). Additionally, the original authors didn't understand the separation between server-side and client-side so the code was mixed all throughout the document pages in nonsensical ways. And worst of all, every page was vulnerable to SQL injection in every single query that would just concatenate user inputs into the SQL command. It wasn't uncommon to find code that didn't even do what it was intended to do.

Another notable example that comes to mind is a Web application that was written in a proprietary IE-only scripting language (I can't remember now if it was VBScript or a superset of JScript or something else entirely). It was developed originally in Spanish too which didn't help. And the code was complete spaghetti. I was asked if I could make changes to it. I wasn't even able to begin to understand how it worked. The person or team that developed it was long gone, perhaps on the other side of the world. Who, what, when, where, why, how? Nobody knew. I didn't know anything about it other than where the code existed in the company network. It was a large international corporation and the code had been shared from a different location. I forget what, but I did manage to make a minor change that I guessed might accomplish a backwards, compromising, partial goal. It was a really big hack, and I couldn't even prove that it worked, but the guy requesting it was excited about it nevertheless...

I don't even know enough to understand what's wrong with Matthew's example

I know you're shitting me, but I'll humor you. The password was stored in a publically accessible database.

Reminds me of the data files from grabber.exe from allegro (< 5.0) that were "password" protected. The passwords were hardcoded as a string. You could easily open the exe in a editor and find all the hardcoded strings.

Name it .dat and you've defeated 3/4 of the world.

Passwords should never be stored in plaintext because they're supposed to be a secret that is only known to the user it belongs to. Ideally, even the host shouldn't be able to determine what a user's password is. The usual solution to this is hashing the password before storing it. You can still verify that a user entered the correct password, but cannot realistically reverse the hash to get the original password nor can you easily get hash collisions with other inputs (this hinges on the quality and complexity of the hash algorithm that you choose).

As an added safety measure you should include a user-specific "salt" string along with the password before hashing it so that the same passwords will have a different hash in the database for each user. That makes it more difficult to detect accounts that share the same password.

It doesn't really matter if the database is public or not. A private database can still be leaked if any person/account with access to the host machine is compromised, or if the network is hacked. Often databases that were supposed to have been private end up leaked. The only way to guard against this is hashing and encrypting (with a key[-pair] stored separately) any data that is sensitive in nature. Passwords, payment info, and/or any other data that is personal in nature.

The database in this case was not public, but there was an unauthenticated, public Web-based API endpoint (e.g., https://example.com/api/users) that outputs all fields in the entire user table. And since the user table contained plaintext passwords and MD5 hashed passwords (a weak algorithm that is easily broken) all of the passwords for the system were exposed to the world... Anybody with the API URL was able to access all credentials.

Literally everything I've ever touched professionally has horrified me in some way.

4 SQL servers with 24 cores (when they FIRST came out, over $30 grand a computer), all running their SQL database off a USB 2.0 5400 RPM HDD. "so they can remove it easily for fire safety."

Gigabit FIBER around the entire compound. Then tied to a T3 line (which is like 300 KB/s) and the sales techs complain "Microsoft Dynamics is really slow when used remotely." Their goddamn CELLPHONES have faster internet.

I felt like Scotty every day. "I'm a programmer, not a MAGICIAN"

More IT, there's a college that lists their entire site's root passwords in a plaintext file available ONLINE so Norton Ghost can retrieve the ISOs when they install new computers. (plz die.)

I haven't even started on the software I had to maintain.