- 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
Alex, we probably didn't work at the same place, unless you have been working in Poland for some time. But perhaps some 'patterns' are common worldwide...
To give credit to the author of above code, I have to say, that it actualy works - and works quite nice. Strange but true. But the machine hosting that application is one of the most powerful I've personaly seen.
And the author is probably proficient at working with cryptic code, because he started doing software development when there was no PC... and when you wanted to connect one of the first hard drives to your strange machine at university you had to build a hardware controller and program it by yourself.
(and, to add some dimension to that application, it uses SQL Server as a back-end and there is administrative interface done in MS Access. It's lime green and 100% yellow in some places. Gross.)
Admin
Someone should teach this programmer the beauty of parameterized queries, for one thing. I can only assume (for example) line 4 and 10 are designed to stop SQL injection - but its a damn strange way to fix it.
Admin
But parametrized queries aren't helpful in this situation. Notice that he uses the strCh variable as a boolean to indicate an SQL Injection attempt and log it into a table bbroth ("Big Brother"?), gives the user a fake message "No Such ID", and then a scary one (at least, for those attempting SQL Injection): "Your details are logged".
Style, names, and false alarms apart, it works.
Admin
Daniel,
what concerns me most about that code, is that is the WHOLE INCLUDE FILE!
So what do we have here:
- a lot of meaningless variables from outside of current include
- a break of processing flow in the include file (is.cl.asp is a footer, then you have response.end) - there are other files when it is done conditionaly 5-10 times in one include
And whole application is done this way. This is a horrible way to modularize code...
Admin
Isn't there a problem on lines 10,11 and 12?
Shouldn't they be pau=replace(pau...?
Still, interesting code to say the least :)
Admin
Will,
you have good eye :)
I haven't noticed this before. Have to send an email to poor soul who is maintaining the code now...
Admin
"But parametrized queries aren't helpful in this situation."
Well, they are, because they'd let you record the attempted injection verbatim, rather than having to guard against injection yourself before inserting into the database.
And, of course, there's no need to guard against this kind of injection /anyway/ if you're using parametrized queries.
Admin
Looks like typical dinosaur programmer's code. I have seen lots of code that looks like that. And it was all written by programmers that started writing code with Algol/ADA/whatever that only allowed variable names up to 8 characters. (or something like that).
Or maybe he's securing his job by writing code that noone else can read/maintain/understand.
Admin
I'm sure that after 50,000 lines of code it'll come more easily to you.
Admin
oof, Logging the incorrect password is a semi-malicious thing to do. I rememebr someone who made a utility site for some SecondLife malware like utility. They logged all incorrect passwords and emails, then later bragged about how they could often log into peoples emails with the password they incorrectly tried to use on his site.