PDA

Click to See Complete Forum and Search --> : Login System

Chris Muench
12-04-2003, 10:01 PM
Are their any major problems you notice with this code: (I have many files because this is just a start to the re write of my program) I just need to know if their are any security issues or bad coding.

login.php
<html>
<head>

</head>

<body>
<?php
session_start();

include ("config.php");
include ("classes/db_functions.php");
include ("classes/security_functions.php");

//create two objects that are needed in this script
$dbf=new db_functions($cfg_server,$cfg_username,$cfg_password,$cfg_database);
$sec=new security_functions();

if(isset($_POST['username']) and isset($_POST['password']))
{
$username = $_POST['username'];
$password = $_POST['password'];

if($sec->checkLogin($dbf,$username,$password))
{
$_SESSION ['user'] = $username;
header ("location: index.php");
}
else
{
echo '<center>username or password are incorrect</center>';
}
}

elseif($sec->isLoggedIn())
{
header ("location: index.php");
}
else
{
?>
<center>
<form name="login" action="login.php" method="POST">
<table width="348" border="0" cellspacing="2" cellpadding="0">
<tr>
<td width="85">
Username:
</td>
<td>
<input type="text" name="username" size="24">
</td>
</tr>
<tr>
<td width="85">
Password:
</td>
<td>
<input type="password" name="password" size="24">
</td>
</tr>
</table>
<input type="submit" name="Submit" value="Log In">
</form>
</center>

<?php
}

?>
</body>
</html>


index.php
<?php
session_start();
include ("config.php");
include ("classes/db_functions.php");
include ("classes/security_functions.php");


$dbf=new db_functions($cfg_server,$cfg_username,$cfg_password,$cfg_database);
$sec=new security_functions();

if(!$sec->isLoggedIn())
{
header ("location: login.php");
}

?>

db_functions.php
<?php
class db_functions
{
//class variable that represents the database connection.
var $conn;

//user-defined constructor
function db_functions($server,$username,$password,$database)
{
//pre: parameters must be correct in order to connect to database.
//post: connects to database.

$this->conn = mysql_connect("$server", "$username", "$password") or die("Could not connect : " . mysql_error());
mysql_select_db("$database",$this->conn) or die("Could not select database <b>$database</b>");

}


}

?>


security_functions.php
<?php


class security_functions
{
//defalt constructor
function security_functions()
{


}

function isLoggedIn()
{
if(isset($_SESSION['user']))
{
return true;
}
return false;
}

function checkLogin($dbf,$username,$password)
{
//pre: $dbf must be a db_functions object and $username and password must be strings
//post: returns boolean based on if their login was succesfull.

$result = mysql_query ("SELECT * FROM users WHERE username='$username' and password='$password'",$dbf->conn);

$num = @mysql_num_rows($result);

if($num > 0)
{
return true;
}

return false;
}



}

Thanks,

Chris Muench
planetsim
12-05-2003, 04:31 AM
Im a bit weary about this function "isLoggedIn() "

You are only checking if the user has a session registered. You should also check that the user session matches maybe a userid or username to make sure they are not only logged in but registered.

Also you must encrypt your passwords either by md5() which just hashes or there is mcrypt not sure on others
Weedpacket
12-05-2003, 05:57 AM
Originally posted by planetsim
Also you must encrypt your passwords either by md5() which just hashes or there is mcrypt not sure on others There's also PHP's native support of SHA1, which happens to be what the NIST recommends, but that's a more recent addition.
Chris Muench
12-05-2003, 07:23 AM
How do you dycrpt passwords inserted into the database (md5)
Weedpacket
12-05-2003, 08:49 AM
Originally posted by Chris Muench
How do you dycrpt passwords inserted into the database (md5) You can't. That's the idea.
Shrike
12-05-2003, 01:53 PM
Then the client asks "can we have a remind password feature please?" :D
planetsim
12-05-2003, 04:52 PM
And you create a random password function


function rnd_Pass()
{
srand(microtime() * 1000000000000);
$pass_rnd = array('a','b','c',
'd','e','f',
'g','h','i',
'j','k','l',
'm','n','o',
'p','q','r',
's','t','u',
'v','w','x',
'y','z','A',
'B','C','D',
'E','F','G',
'H','I','J',
'K','L','M',
'N','O','P',
'Q','R','S',
'T','U','V',
'W','X','Y',
'Z','0','1',
'2','3','4',
'5','6','7',
'8','9');
$rnd = array_rand($pass_rnd, 8);
$password = "";
for($i=0;$i<=7;$i++){
$password .= $pass_rnd[$rnd[$i]];
}
return $password;
}


Quite a basic one there
agoe
12-05-2003, 07:52 PM
Originally posted by planetsim
Im a bit weary about this function "isLoggedIn() "

You are only checking if the user has a session registered. You should also check that the user session matches maybe a userid or username to make sure they are not only logged in but registered.


i disagree. if the user types the correct username and password, the session-variable is set. otherwise it is not.

consider this:

$sql = "SELECT the, fields, you, need FROM your_table WHERE Username='".validiate_function($_POST['username'])."' AND Password='".md5($_POST["password"])."'";
$result = mysql_query($sql) or die(mysql_error());

if (mysql_num_rows($result) == 1) {
// the user is authenticated
$_SESSION['authed'] = 1;
} else {
echo "wrong username/password";
}


then, on top of every page that needs authentication, just check if $_SESSION['authed'] == 1.

it's as bulletproof as it gets.
Moonglobe
12-05-2003, 08:22 PM
$pass_rnd = array('a','b','c',
'd','e','f',
'g','h','i',
'j','k','l',
'm','n','o',
'p','q','r',
's','t','u',
'v','w','x',
'y','z','A',
'B','C','D',
'E','F','G',
'H','I','J',
'K','L','M',
'N','O','P',
'Q','R','S',
'T','U','V',
'W','X','Y',
'Z','0','1',
'2','3','4',
'5','6','7',
'8','9'); $pass_rnd = array_merge(range('a','z'), range('A','Z'), range(0,9));
daveyboy
12-15-2003, 03:39 PM
checking a single session value is fine, as long as u only use it for authentication. whenever you call session_start() your session vars will overwrite any the user has passed in. just make sure that you're checking $_SESSION['user'] not just $user.

one thing: your redirects are incorrect: the location header requires a fully qualified url, e.g. http://www.example.com/index.php
Moonglobe
12-15-2003, 08:00 PM
Originally posted by daveyboy
one thing: your redirects are incorrect: the location header requires a fully qualified url, e.g. http://www.example.com/index.php in theory yes, but there's near total backward compatibiliy with HTTP/1.0 (that's a 1.1 thing)
planetsim
12-15-2003, 08:18 PM
Originally posted by Moonglobe
$pass_rnd = array_merge(range('a','z'), range('A','Z'), range(0,9));

Suppose should update my code.. I remember writing that function like when i first learnt i thought it was the best thing i made and never went and made it better.