Comment On Keeping It Stupid Simple

"Not too long ago," writes R.S., "a coworker left the company for greener pastures. His motto was Keep It Simple and, until now, I wasn't sure how simple he liked things." [expand full text]
« PrevPage 1 | Page 2Next »

Re: Keeping It Stupid Simple

2008-09-15 08:04 • by SteveB (unregistered)
If only he'd read last week's post on SQL injection...

Re: Keeping It Stupid Simple

2008-09-15 08:05 • by amischiefr
Universal Password!!! Brilliant!

Re: Keeping It Stupid Simple

2008-09-15 08:05 • by snoofle
So there is no key and the column "username" has the password.

Methinks the coder was "simple"...

Re: Keeping It Stupid Simple

2008-09-15 08:06 • by Xeron
If i had goggles, they would do NOTHING.

Re: Keeping It Stupid Simple

2008-09-15 08:07 • by Buggz
I can't remember having seen a stronger invitation for a brute force attack. Logging in using a single piece of input..

Re: Keeping It Stupid Simple

2008-09-15 08:10 • by notme (unregistered)
217355 in reply to 217352
snoofle:
So there is no key and the column "username" has the password.

Methinks the coder was "simple"...


I think TRWTF was that the actual authentication consisted of redirecting the user to index.php (not exactly a hard to guess name, either). He didn't even note down a session variable saying "the user belonging to this session is authenticated" or something to that effect.

Since the index.php is the page that is normally served by default, I wouldn't be surprised if even non-tech-savvy users could "hack" the site - "hack" as in they won't even notice the site was supposed to be password protected.

Re: Keeping It Stupid Simple

2008-09-15 08:12 • by ph (unregistered)
So I get it right:

He runs through all usernames, and the last username in the list (in whatever order SELECT returns them) is the password?

I am impressed.

Re: Keeping It Stupid Simple

2008-09-15 08:15 • by snoofle
217358 in reply to 217356
ph:
So I get it right:

He runs through all usernames, and the last username in the list (in whatever order SELECT returns them) is the password?

I am impressed.
That's what I thought; then I read the last line of the post!

Re: Keeping It Stupid Simple

2008-09-15 08:17 • by summerian (unregistered)
217359 in reply to 217358
He should be the dictionary definition of 'code monkey'.

Re: Keeping It Stupid Simple

2008-09-15 08:17 • by Simple User (unregistered)
Not only is it simple, it is Easy To Use (TM). I so hate it when I have to log in to a site, navigate 20 clicks through some obscure trail, and when I finally get to my favorite page I can't bookmark it because next time the site forces me through all those convoluted steps again!

This site supports bookmarks! Just login once and no revalidation next time! No wonder he got hired by another company, probably a bank or someone with lots of money to burn.

On second thought, couldn't be a bank. They couldn't work out how to make a site usable if it was the last step between them and a Congressional bailout!

Re: Keeping It Stupid Simple

2008-09-15 08:17 • by ParkinT
Perhaps the application never gained more than one user.
Plenty of room for growth!!

Re: Keeping It Stupid Simple

2008-09-15 08:18 • by Gorfblot (unregistered)
217362 in reply to 217356
ph:
So I get it right:

He runs through all usernames, and the last username in the list (in whatever order SELECT returns them) is the password?

I am impressed.


Right. You would think this is a WTF, as SELECT without an ORDER BY clause does not guarantee order (As you stated). Except, as there is only a single username in that table, the order is relatively predictable.

Re: Keeping It Stupid Simple

2008-09-15 08:34 • by Smash King
I like how descriptive he gets when naming his variables. Who wouldn't know the meaning of $q and $chumbawumba ?

Re: Keeping It Stupid Simple

2008-09-15 08:37 • by BK (unregistered)
Classic example of bad naming. Had the table been named SingleWebPassword and its single column [password] , there wouldn't be WTF

Re: Keeping It Stupid Simple

2008-09-15 08:42 • by Stephen Bayer (unregistered)
I call bs on this one.. no one would be that stupid.

Re: Keeping It Stupid Simple

2008-09-15 08:47 • by Smash King
217367 in reply to 217366
Stephen Bayer:
I call bs on this one.. no one would be that stupid.
You must be new here. Welcome.

Re: Keeping It Stupid Simple

2008-09-15 08:48 • by java.lang.Chris;
This is exactly why my company rejects job applications from anyone who listens to Chumbawumba. Or the Scissor Sisters. You just can't trust them.

Re: Keeping It Stupid Simple

2008-09-15 08:48 • by dpm
217369 in reply to 217366
Stephen Bayer:
I call bs on this one.. no one would be that stupid.
Thank you, I needed a good hard laugh and the OP did not supply one, but you came through.

Re: Keeping It Stupid Simple

2008-09-15 08:50 • by \ (unregistered)
217370 in reply to 217350
SteveB:
If only he'd read last week's post on SQL injection...


If only last week's post read this - no user input gets sent to the query, 100% SQL Injection Proof!

I'll be using this in all my endeavours.

Re: Keeping It Stupid Simple

2008-09-15 08:53 • by JimM
217371 in reply to 217359
summerian:
He should be the dictionary definition of 'code monkey'.
Nononono - he's only the dictionary definition of a very specific use of 'code monkey'. And frankly, I'd hoped a trained monkey wouldn't write code that bad...

On the other hand, it does avoid the risk of SQL injection attacks... ;^) EDIT: Goodness, only three other people mentioned that whilst I was posting...
Stephen Bayer:
I call bs on this one.. no one would be that stupid.
You're joking. Right? Please tell me you're joking?!? You MUST BE JOKING!?!?!?!?!?

Re: Keeping It Stupid Simple

2008-09-15 08:55 • by java.lang.Chris;
217372 in reply to 217366
Stephen Bayer:
I call bs on this one.. no one would be that stupid.


Nope. In a previous system I worked on, the original coder did the following to try and login a user:


boolean login(String username, String password) {

Statement s = c.createStatement();

ResultSet rs = s.executeQuery("SELECT username, password FROM account");

while (rs.next()) {
if (rs.getString(1).equals(username) && rs.getString(2).equals(password)) {
return true;
}
}

return false;
}


The account table had 1.7 million entries, and there was a perfectly good index on the username column. Logging in was slow, and he leaked connections by not closing them - if the database ran out of connections before the garbage collector kicked in he was straight outta luck.

Re: Keeping It Stupid Simple

2008-09-15 09:00 • by JimM
217373 in reply to 217372
java.lang.Chris;:
Stephen Bayer:
I call bs on this one.. no one would be that stupid.

The account table had 1.7 million entries, and there was a perfectly good index on the username column. Logging in was slow, and he leaked connections by not closing them - if the database ran out of connections before the garbage collector kicked in he was straight outta luck.
On the other hand: SQL Injection proof, AND it checks both the username and password! In the world of WTF, that's a WIN, surely? And we've all seen worse:
<input type="button" onclick='

if(document.getElementById("txtpass").value == "mypassword")
document.location="mysecretpage.html"' />

Re: Keeping It Stupid Simple

2008-09-15 09:01 • by itsmo (unregistered)
217374 in reply to 217351
amischiefr:
Universal Password!!! Brilliant!


You mean Brillant!

Re: Keeping It Stupid Simple

2008-09-15 09:06 • by eehoo
The real WTF here is the (common) misspelling of Chumbawamba. Who put the U in the wamba?

Re: Keeping It Stupid Simple

2008-09-15 09:08 • by Rob (unregistered)
217376 in reply to 217350
SteveB:
If only he'd read last week's post on SQL injection...


Where do you see the possibility for SQL injection in this code?

Re: Keeping It Stupid Simple

2008-09-15 09:09 • by noone (unregistered)
217377 in reply to 217368
java.lang.Chris;:
This is exactly why my company rejects job applications from anyone who listens to Chumbawumba. Or the Scissor Sisters. You just can't trust them.

I got knocked down by your company.
But I got up again.
You weren't ever gonna keep me down.

Re: Keeping It Stupid Simple

2008-09-15 09:13 • by Bejesus (unregistered)
Sadly, that's significantly more elegant than most Single Sign On solutions I've encountered...

Re: Keeping It Stupid Simple

2008-09-15 09:21 • by Bill (unregistered)
Don't knock down this developer. He'll just get back up again, you're never gonna keep him down.

Re: Keeping It Stupid Simple

2008-09-15 09:27 • by summerian (unregistered)
217380 in reply to 217371
JimM:
Nononono - he's only the dictionary definition of a very specific use of 'code monkey'. And frankly, I'd hoped a trained monkey wouldn't write code that bad...


I must agree. Perhaps there should be another term - sth like 'code baboon' ...

Re: Keeping It Stupid Simple

2008-09-15 09:27 • by silent d (unregistered)
I get logged out, but I log in again, you're never gonna keep me out...

Re: Keeping It Stupid Simple

2008-09-15 09:29 • by BrianK (unregistered)
217382 in reply to 217350
To quote a great movie... "INCONCEIVABLE!"

Re: Keeping It Stupid Simple

2008-09-15 09:32 • by Ken B (unregistered)
217383 in reply to 217374
itsmo:
amischiefr:
Universal Password!!! Brilliant!
You mean Brillant!
No, he meant "Universal Pbuttword".

Re: Keeping It Stupid Simple

2008-09-15 09:33 • by powerlord
217385 in reply to 217378
Bejesus:
Sadly, that's significantly more elegant than most Single Sign On solutions I've encountered...

Interesting definition of "Single Sign On," though.

Re: Keeping It Stupid Simple

2008-09-15 09:34 • by Smash King
217386 in reply to 217375
eehoo:
The real WTF here is the (common) misspelling of Chumbawamba. Who put the U in the wamba?
The same coder we're bashing did.

And I don't think it is nowhere near being the real WTF here

Re: Keeping It Stupid Simple

2008-09-15 09:34 • by java.lang.Chris;
217387 in reply to 217373
JimM:
java.lang.Chris;:
Stephen Bayer:
I call bs on this one.. no one would be that stupid.

The account table had 1.7 million entries, and there was a perfectly good index on the username column. Logging in was slow, and he leaked connections by not closing them - if the database ran out of connections before the garbage collector kicked in he was straight outta luck.
On the other hand: SQL Injection proof, AND it checks both the username and password! In the world of WTF, that's a WIN, surely? And we've all seen worse ...


The reason it came to mind was the same programmers refusal to use parameterised statements. I spent a long time demonstrating an SQL injection vulnerability to him, and how to avoid them with the JDBC PreparedStatement. However, all that happened was that his code went from:


Statement s = c.createStatement();
s.executeQuery("UPDATE foo SET x = " + userSuppliedString);


to:


PreparedStatement s = c.prepareStatement("UPDATE foo SET x = " + userSuppliedString);


At which point I refused to have him work on any of my projects.

Re: Keeping It Stupid Simple

2008-09-15 09:35 • by I walked the dinosaur (unregistered)
217388 in reply to 217381
silent d:
I get logged out, but I log in again, you're never gonna keep me out...


LOL!!!!!!

Re: Keeping It Stupid Simple

2008-09-15 09:38 • by I walked the dinosaur (unregistered)
217389 in reply to 217388
Pissing the WHERE away, pissing the WHERE away!

Re: Keeping It Stupid Simple

2008-09-15 09:39 • by mauhiz (unregistered)
Select commment from COMMENTS;

Re: Keeping It Stupid Simple

2008-09-15 09:45 • by fruey
217391 in reply to 217390
mauhiz:
Select commment from COMMENTS;


Shouldn't that read

SELECT comment FROM comments WHERE grey_background = TRUE;

Re: Keeping It Stupid Simple

2008-09-15 09:46 • by DaveK
217392 in reply to 217372
java.lang.Chris;:

Logging in was slow, and he leaked connections by not closing them - if the database ran out of connections before the garbage collector kicked in he was straight outta luck.
I do not believe the "S" in "SOL" means what you think it means.

Re: Keeping It Stupid Simple

2008-09-15 09:48 • by Aaron
I'm guessing that it started out with a hard-coded password (possibly even some pathetic JavaScript validation), and somebody told him he should be using a database for authentication, and he didn't really understand what that meant but figured he'd give it the old college try.

Re: Keeping It Stupid Simple

2008-09-15 09:53 • by DaveK
217395 in reply to 217375
eehoo:
The real WTF here is the (common) misspelling of Chumbawamba. Who put the U in the wamba?


JAM TIME! And a-one, and a-two, and a one-two-three-four :

Who put the bomp in the bomp-a-bomp-a-bomp
Who put the ram in the ram-a-lam-a-ding-dong
Who put the bop in the bop-she-bop-she-bop
Who put the dip in the dip-de-dip-de-dip
Take it, eehoo!
eehoo:
Who put the U in the wa-a-amba?
Who was that man, I'd like to shake his hand
...


Re: Keeping It Stupid Simple

2008-09-15 09:57 • by anne (unregistered)
He didn't even spell "Chumbawamba" right.

Re: Keeping It Stupid Simple

2008-09-15 10:07 • by delenit (unregistered)
217398 in reply to 217391
fruey:
mauhiz:
Select commment from COMMENTS;


Shouldn't that read

SELECT comment FROM comments WHERE grey_background = TRUE;


Don't be silly
rs = execute("SELECT * FROM comments");
while(rs.next()) {
if(rs.get("grey_background") != FILE_NOT_FOUND){
println(rs);
}
}

Re: Keeping It Stupid Simple

2008-09-15 10:11 • by Nobody (unregistered)
Forgeting the login part for a moment I recognize this pattern. Using a table with one record as a kind of substitute for an Application variable.

I see this more often than I'd like.

Re: Keeping It Stupid Simple

2008-09-15 10:11 • by Meep3d
$q->query($sql);

The real WTF, and something I see every day, is why just about everyone feels the need to abstract the mysql functions behind another layer of obfuscation?

I've never heard a convincing reason for this and no doubt the first thing you have to do when looking at someone elses code is have to figure out the bizaare idiosyncrasies of their particular reimplementation.

Re: Keeping It Stupid Simple

2008-09-15 10:18 • by M (unregistered)
217401 in reply to 217366
Stephen Bayer:
I call bs on this one.. no one would be that stupid.

I've seen something almost as bad (single user, username and password set to the same string, which was the three-letter acronym for the organization.) The username/password was not intended to provide strong security, just to prevent casual access. The app was internal and read-only, so not as bad as it might sound.

Re: Keeping It Stupid Simple

2008-09-15 10:32 • by Code Dependent
217402 in reply to 217399
Nobody:
Forgeting the login part for a moment I recognize this pattern.
The Simpleton pattern?

Re: Keeping It Stupid Simple

2008-09-15 10:39 • by java.lang.Chris;
217403 in reply to 217392
DaveK:
java.lang.Chris;:

Logging in was slow, and he leaked connections by not closing them - if the database ran out of connections before the garbage collector kicked in he was straight outta luck.
I do not believe the "S" in "SOL" means what you think it means.



Damn shit it does.

Re: Keeping It Stupid Simple

2008-09-15 10:44 • by Kermos
217404 in reply to 217400
Meep3d:
$q->query($sql);

The real WTF, and something I see every day, is why just about everyone feels the need to abstract the mysql functions behind another layer of obfuscation?

I've never heard a convincing reason for this and no doubt the first thing you have to do when looking at someone elses code is have to figure out the bizaare idiosyncrasies of their particular reimplementation.


One convincing reason is that if you ever need to switch databases from MySQL to something else you don't have to rewrite all your code. Assuming that the SQL code is compatible you'd only need to rewrite the class that handles the API access.

I also do it to simplify a few things. If for instance I know for an undeniable fact I will only receive a single value from a query then my database class has a function that can do exactly that: Return a single value from any given query.

That takes a whole bunch of MySql function calls and reduces it to a single call from the application. Makes my life a whole lot easier.

It also simplies cleanup as the moment my query class goes out of scope it is properly disposed of by the destructor. Same goes for database connections.

So yea, there are a few reasons to put the API behind another layer. Hopefully however this layer is a layer that makes life easier, not obfuscates things.

« PrevPage 1 | Page 2Next »

Add Comment