Is my logon and access control code secure?
Posted: Thu Aug 20, 2009 12:48 pm
I've recently coded a small website from scratch to learn php. I've also just read something about how you shouldn't try writing your own access control systems because you will screw up -which makes me worried.
My Pseudocode:
Logging in
Basically, to login a user I grab their escaped user alias and password from a form. I pass these variables to a sql query that tries to select the user alias from the database with the given parameters. If the selected alias matches the alias given in the form, the login is successful and I set a bunch of session variables using $_SESSION['myvariables'] . I am currently not using authentication headers (is that a baaad thing?)
Access Control
Now for the access control: All my pages are called from an index.php page. At the beginning of my index.php file and thus at the beginning of each of my pages, I use the sessioned alias and password to select the users authorization level from the database. This will return 0 for a guest (sessioned variables not found), 1 for a normal user, and 10 or above for an admin. I pass this authorization level through to the rest of my code, including my html SMARTY templates to display the appropriate data. For any action that involves an Insert, or Update (I dont have any queries that perform deletes) I run the function that checks the authorization level and make sure we have the required auth level to perform whatever action we're doing (If the check fails and the auth level does not meet the minimum for that action, the user is sent to a denied page).
Whew. Okay, heres my code:
Login Code:
Access Control: Determining Users Authorization Level:
All my input variables are escaped with mysql_real_escape before being passed to the functions. I wasn't sure if generally it was better to escape inside the functions, or to have naive functions but escape everything as soon as you get it from the user higher up in your code. I went for the latter because I wanted to escape the user input as soon as I got it from them.
EDIT: I just read some answers to similar questions. I will rewrite the code so that I am not storing the password in a session variable.
My Pseudocode:
Logging in
Basically, to login a user I grab their escaped user alias and password from a form. I pass these variables to a sql query that tries to select the user alias from the database with the given parameters. If the selected alias matches the alias given in the form, the login is successful and I set a bunch of session variables using $_SESSION['myvariables'] . I am currently not using authentication headers (is that a baaad thing?)
Access Control
Now for the access control: All my pages are called from an index.php page. At the beginning of my index.php file and thus at the beginning of each of my pages, I use the sessioned alias and password to select the users authorization level from the database. This will return 0 for a guest (sessioned variables not found), 1 for a normal user, and 10 or above for an admin. I pass this authorization level through to the rest of my code, including my html SMARTY templates to display the appropriate data. For any action that involves an Insert, or Update (I dont have any queries that perform deletes) I run the function that checks the authorization level and make sure we have the required auth level to perform whatever action we're doing (If the check fails and the auth level does not meet the minimum for that action, the user is sent to a denied page).
Whew. Okay, heres my code:
Login Code:
Code: Select all
function loginUser($userAlias, $userPassword) {
$userAlias = escape($userAlias);
$userPassword = escape($userPassword);
$this->Alias = $userAlias;
$this->Password = $userPassword;
$sql_login= " SELECT tu.UserID, tu.UserAlias FROM $this->db.$this->table tu
WHERE tu.UserAlias = '$this->Alias' AND tu.UserPassword = '$this->Password' ; ";
$result = mysql_query($sql_login);
$row = mysql_fetch_row($result);
if ( $this->Alias == '' || $row[1] != $this->Alias) {
// logon fails
$_SESSION['log_sec'] = 0;
$_SESSION['reg_fail_alias'] = $this->Alias;
$_SESSION['logged_in'] = 0;
$_SESSION['failed_login'] = 1;
return 0;
} else {
// logon succeeds
$_SESSION['log_sec'] = 1;
$_SESSION['session_alias'] = $this->Alias;
$_SESSION['session_alias_id'] = $row[0];
$_SESSION['session_password'] = $this->Password;
$_SESSION['failed_login'] = 0;
$_SESSION['reg_fail_alias'] = '';
sleep(1);
return 1;
}
}
Code: Select all
function getAuth() {
$alias = $_SESSION['session_alias'];
$passw = $_SESSION['session_password'];
if ( isset($alias) && isset($passw) ) {
$sql = " SELECT AuthLevel FROM $this->db.$this->table WHERE UserAlias = '$alias'
AND UserPassword = '$passw' ; ";
$result = mysql_query($sql);
$returned_rows = mysql_num_rows ($result);
if ( $returned_rows == 0 ) {
$auth = 0; // (hacker or session vars no longer match db)
} else {
$row = mysql_fetch_row($result);
$auth = $row[0] ;
}
} else { // we're not logged in, auth = 0.
$auth = 0;
}
return $auth; //I thenpass
}
EDIT: I just read some answers to similar questions. I will rewrite the code so that I am not storing the password in a session variable.