Allegro.cc - Online Community

Allegro.cc Forums » Off-Topic Ordeals » what's the worst professional code you've ever seen?

This thread is locked; no one can reply to it. rss feed Print
what's the worst professional code you've ever seen?
Matthew Leverton
Supreme Loser
January 1999
avatar

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. :o 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. ;D

RmBeer2
Member #16,660
April 2017
avatar

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.

🌈🌈🌈 🌟 BlackRook WebSite (Only valid from my installer) 🌟 C/C++ 🌟 GNU/Linux 🌟 IceCream/Cornet 🌟 🌈🌈🌈

Rm Beer for Emperor 2021! Rm Beer for Ruinous Slave Drained 2022! Rm Beer for Traveler From The Future Warning Not To Enter In 2023! Rm Beer are building a travel machine for Go Back from 2023! Rm Beer in an apocalyptic world burning hordes of Zombies in 2024!

Edgar Reynaldo
Major Reynaldo
May 2007
avatar

RmBeer2
Member #16,660
April 2017
avatar

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

🌈🌈🌈 🌟 BlackRook WebSite (Only valid from my installer) 🌟 C/C++ 🌟 GNU/Linux 🌟 IceCream/Cornet 🌟 🌈🌈🌈

Rm Beer for Emperor 2021! Rm Beer for Ruinous Slave Drained 2022! Rm Beer for Traveler From The Future Warning Not To Enter In 2023! Rm Beer are building a travel machine for Go Back from 2023! Rm Beer in an apocalyptic world burning hordes of Zombies in 2024!

Mark Oates
Member #1,146
March 2001
avatar

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. :o You have missed the point. Entirely.

I enjoyed this ;D.

Nah, it's hashed bro 8-)

--
Visit CLUBCATT.com for cat shirts, cat mugs, puzzles, art and more <-- coupon code ALLEGRO4LIFE at checkout and get $3 off any order of 3 or more items!

AllegroFlare β€’ AllegroFlare Docs β€’ AllegroFlare GitHub

Polybios
Member #12,293
October 2010

Wow. :o

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. ;D

Bob
Free Market Evangelist
September 2000
avatar

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.

--
- Bob
[ -- All my signature links are 404 -- ]

DanielH
Member #934
January 2001
avatar

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.

William Labbett
Member #4,486
March 2004
avatar

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.

jmasterx
Member #11,410
October 2009

My code's quite good because I avoid the need for header guards

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

Or ForwardDeclareParty ?

William Labbett
Member #4,486
March 2004
avatar

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.

BitCruncher
Member #11,279
August 2009
avatar

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.

jmasterx
Member #11,410
October 2009

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

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 ;D

bamccaig
Member #7,536
July 2006
avatar

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...

Peter Hull
Member #1,136
March 2001

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

Edgar Reynaldo
Major Reynaldo
May 2007
avatar

DanielH
Member #934
January 2001
avatar

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.

Edgar Reynaldo
Major Reynaldo
May 2007
avatar

bamccaig
Member #7,536
July 2006
avatar

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. :P 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. :P

Chris Katko
Member #1,881
January 2002
avatar

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.

-----sig:
β€œPrograms should be written for people to read, and only incidentally for machines to execute.” - Structure and Interpretation of Computer Programs
"Political Correctness is fascism disguised as manners" --George Carlin

Go to: