Page 1 of 1

My Basic Log In Script, any security advices?

Posted: Fri Dec 08, 2006 7:24 am
by kaisellgren
Hi there all!

My current trimmed login script is the following:

Code: Select all

<?php

session_start();

require("mysql.php"); // Requires a mysql class
$access = false; // I do not have register globals enabled

if (isset($_COOKIE["username"]) && isset($_COOKIE["password"]))
 {
  $username = escape($_COOKIE["username"]); // Escape is my escaping function, don't care about it
  $password = escape($_COOKIE["password"]);
  if (is_valid_up($username) && is_valid_up($password)) // is_valid_Up checks if username/password is valid format
   {
    $db = new mysql;
    $db -> connect();
    $db -> query("SELECT * FROM ".PREFIX."members WHERE (username='$username' AND password='$password');");
    if ($db -> num_rows())
     {
      $access = true;
     }
   }
 }
elseif (isset($_POST["log_in"]))
 {
  $username = escape($_POST["username"]); // Escape is my escaping function, don't care about it
  $password = escape(secure($_POST["password"])); // The secure function hashes the argument and then returns it as hashed
  if (is_valid_up($username) && is_valid_up($password)) // is_valid_Up checks if username/password is valid format
   {
    $db = new mysql;
    $db -> connect();
    $db -> query("SELECT * FROM ".PREFIX."members WHERE (username='$username' AND password='$password');");
    if ($db -> num_rows())
     {
      $access = true;
     }
   }
 }

if ($access)
 // show member data
else
 // dont show member data

?>
By trimmed I mean that I delete unnecessary code you won't gonna need to know.

Basically I came to here to ask help about security. Most of you say that storing password in cookies is bad etc. How would the script log in automatically then using cookies?

Thank you for your help.

Posted: Fri Dec 08, 2006 8:36 am
by Ollie Saunders
btw there is a security forum specifically for this kind of stuff.

Automatic login via cookies is always going to be insecure. Not to mention more work actually. I have never ever used a cookie directly in almost 2 years of PHP usage because sessions are a much better, more secure, alternative.

Also google "SQL injection" your script looks vulnerable.

Posted: Fri Dec 08, 2006 9:04 am
by kaisellgren
ole wrote:btw there is a security forum specifically for this kind of stuff.

Automatic login via cookies is always going to be insecure. Not to mention more work actually. I have never ever used a cookie directly in almost 2 years of PHP usage because sessions are a much better, more secure, alternative.

Also google "SQL injection" your script looks vulnerable.
Maybe you are not using, but I am using with this forum. Everytime I come to devnetwork.net then I'll redirected to forums and automatically logged in and that's good. I would like to make the same for my system.

What do you mean vulnerable?

Posted: Fri Dec 08, 2006 6:11 pm
by evilchris2003
meaning that hackers can get at your script and replace code by injecting their own into it's place

also if your planning on using cookie for login authentication i would use something other than the password

username and say first name (or some other unique identifier) would be much safer

Posted: Fri Dec 08, 2006 9:56 pm
by wyrmmage
yep...make a string out of, say, the user's id, username, and password. Has that string, and store that in your cookie....that makes it almost impossible to hack, really :) (well, it makes the cookies almost impossible to hack, you still need to have secure script besides them :P)
-wyrmmage

Posted: Fri Dec 08, 2006 10:46 pm
by feyd
Storing the username and password in a cookie is silly. It is no more secure than posting those values on a non-SSL page, except you would be posting those to every single page requested that satisfied the domain and path properties.

Posted: Sat Dec 09, 2006 5:00 am
by kaisellgren
I have currently the script storing User ID and hashed Password in the cookies. I did it so because I noticed a lot of IPB boards using the same method. So IPBs are vulnerable or insecure?

Posted: Sat Dec 09, 2006 5:20 am
by onion2k
kaisellgren wrote:I have currently the script storing User ID and hashed Password in the cookies. I did it so because I noticed a lot of IPB boards using the same method. So IPBs are vulnerable or insecure?
Your variable names should reflect what they actually contain. Calling something $_COOKIE['password'] when it actually contains a hashed copy of the password could confuse anyone who comes to maintain/use your code later.

Posted: Sat Dec 09, 2006 6:19 am
by Ollie Saunders
What do you mean vulnerable?
Vulnerable to SQL injection attack from a uber 1337 haXX0r using a keyboard to make you look a fool with cherries on top.

Posted: Sat Dec 09, 2006 6:33 am
by kaisellgren
ole wrote:
What do you mean vulnerable?
Vulnerable to SQL injection attack from a uber 1337 haXX0r using a keyboard to make you look a fool with cherries on top.
Thank you for the teachnings lol :D

I know what you mean by SQL injection, I meant that how my script is vulnerable for those?

Posted: Sat Dec 09, 2006 6:39 am
by Ollie Saunders

Code: Select all

query("SELECT * FROM ".PREFIX."members WHERE (username='$username' AND password='$password');");
Imagine if

Code: Select all

$password = "' OR 1 = 1); --';
Result:

Code: Select all

SELECT * FROM mehmembers WHERE (username='$username' AND password='' OR 1 = 1);-- ' AND password='$password');
Result: wee! I'm in and I didn't have to know the password

Posted: Sat Dec 09, 2006 6:47 am
by kaisellgren
ole wrote:

Code: Select all

query("SELECT * FROM ".PREFIX."members WHERE (username='$username' AND password='$password');");
Imagine if

Code: Select all

$password = "' OR 1 = 1); --';
Result:

Code: Select all

SELECT * FROM mehmembers WHERE (username='$username' AND password='' OR 1 = 1);-- ' AND password='$password');
Result: wee! I'm in and I didn't have to know the password
Kröhm...

Quote:

Code: Select all

$username = escape($_COOKIE["username"]); // Escape is my escaping function, don't care about it
So, if

Code: Select all

$password = "' OR 1 = 1); --'";
Then:

Code: Select all

SELECT * FROM members WHERE (username='$username' AND password='\' OR 1 = 1); --\'');
And voilá! It will return 0 matches unless someone with $username has password like ' OR 1 = 1); --'....

Posted: Sat Dec 09, 2006 6:53 am
by Ollie Saunders
Oh right. Sorry didn't spot that.

Posted: Sat Dec 09, 2006 7:02 am
by kaisellgren
ole wrote:Oh right. Sorry didn't spot that.
Yeah whatever.

To not go to offtopic, I'll mention again that I was searching for code improvements in my security code. I'm using cookies that store UserID and hashed password. Can someone tell me that is this A) Secure or B) Insecure ? If this is insecure, then what method I should use to able the automatic logins I want.

I understand that storing even hashed password in cookies makes it possible that anyone can get them for example from public computer or hacking into someone's computer. If smurf A gets the userid and password, he can make manually cookie and insert these details to login. The smurf, however, can not change password or other important details without knowing the unhashed password. The smurf can post messages and pretent to be someone else, but this is the failure of the someone who either has unsecure computer or used automatic logins in public computer. Right?