Why do people code so complicatedly?

[Nixon]

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 Code:


<?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]

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]

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:

PHP Code:

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

PHP Code:

$x = ($y != 0) ? 1/$y : $y/1;


is functionally the same as

PHP Code:

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]

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]

What do you mean by scubbing data?

Thanks.

[seby]

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]

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

[drawmack]

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

[steadyguy]

ahundiak ... made my point pretty well didn't I? 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.

[dustbuster]

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]

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]

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]

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]

Okay thanks

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