[dhoyos]

Newbie making a login script so I figured I would put it up so that people could help me out by poining the security holes in it. Im working on encrypting the passwords allthough i don't know how yet. hopefully it isnt too bad :-) Thanks!

<?php
$ip="";
function GetIP(){
global $ip;
if (getenv("HTTP_CLIENT_IP")){
$ip = getenv("HTTP_CLIENT_IP");
}
else if(getenv("HTTP_X_FORWARDED_FOR")){
$ip = getenv("HTTP_X_FORWARDED_FOR");
}
else if(getenv("REMOTE_ADDR")){
$ip = getenv("REMOTE_ADDR");
}
else {$ip = "UNKNOWN";}
return $ip;

}
session_start();
if(isset($username)){
require('auth.php');
$result = mysql_query("SELECT * FROM users WHERE user='$username'",$db) or die(mysql_error());
$numofrows = mysql_num_rows($result);
if ($numofrows==1){
$check = mysql_fetch_array($result);
if($password=$check[pwd]){
session_register("User");
$User=$username;
GetIP();
$SID=session_id();
$result = mysql_query("UPDATE users SET session ='$SID', ip='$ip' WHERE user='$User'",$db) or die(mysql_error());
header("Location: ../login/client.php");
}
}
}
?>

Ommited the login form as allit does is have two inputs and the submit which then comes back to this pae by php self.

once it is logged in it moves on to client.php as header sets it and then this is the begining of client.php


<?php
require('auth.php');
function login(){
header("Location: ../login/login.php");
}
$ip="";
function GetIP(){
global $ip;
if (getenv("HTTP_CLIENT_IP")){
$ip = getenv("HTTP_CLIENT_IP");
}
else if(getenv("HTTP_X_FORWARDED_FOR")){
$ip = getenv("HTTP_X_FORWARDED_FOR");
}
else if(getenv("REMOTE_ADDR")){
$ip = getenv("REMOTE_ADDR");
}
else {$ip = "UNKNOWN";}
return $ip;

}
session_start();
GetIP();
if(isset($User)){
$result = mysql_query("SELECT * FROM users WHERE user='$User'",$db);
$check = mysql_fetch_array($result);
if (session_id()==$check[session] && $ip==$check[ip]){
?>
##!!#!! BLA BLA BLA WEB PAGE##!!#!!
<?php
}else{
login();
}}else{ login();}


?>

[WhSox21]

Nothing to say about the secure part of the script. But I'd suggest when you write code to make it more readable. I read somewhere that it is becoming a standard (or already is) to indent 4 spaces. You should do this everytime you get an if statement or any type or loop or anything like that really. Just my $.02

[sundialsvcs]

Store passwords as the MD5 hash of the password, not the value.

Be sure that all connections are secure; ignore any that aren't, and redirect the user to a secure URL.

If your server connection is SSL-encrypted (and you check!), then you can be reasonably assured of privacy across the net, at least for data of the level of security any of us are likely to have to deal with... (i.e. If you've got a TS-Crypto clearance then presumably you've also already been trained already on the correct procedures for that sort of thing... )

{Hi there, Tom Ridge! How's the kids? No, sorry, this is not a terrorist post. Better luck next time...} ... ... ... (Ooh, I hope those boys have a sense of humor.)