PHP is this code secure?
I found the following code in a previous question on SO. In following code, if the username and password supplied by the user is correct, the user_id and username is stored in session to keep it logged. My question is, why there is need to keep user_id in the session? Isnt only one thing (for example, username) enough to store in session? If the remember is enabled, then a cookie is set, only with username. Now my question is, Is Only username cookie enough? Can't anyone just edit or add the cookie in the browser and log in the system?
Thanks for your replies.
<?
public function login($username, $pass, $remember) {
// check username and password with db
$result = $conn->query("select * from login where
username='".$usern开发者_JAVA技巧ame."' and
password=sha1('".$pass."')");
if (!$result) {
throw new depException('Incorrect username and password combination. Please try again.');
}
if ($result->num_rows>0) {
$row = $result->fetch_assoc();
$_SESSION['user_id'] = $row[user_id];
$_SESSION['username'] = $username;
// start rememberMe
$cookie_name = 'db_auth';
$cookie_time = (3600 * 24 * 30);*/ // 30 days
// check to see if user checked box
if ($remember) {
setcookie ($cookie_name, 'username='.$username, time()+$cookie_time);
}
// If all goes well redirect user to their homepage.
header('Location: http://localhost/v6/home/index.php');
} else {
throw new depException('Could not log you in.');
}
}
?>
THIS CODE IS NOT SECURE! (Sorry for the caps, but its for the emphasis). The SQL statement is susceptible to SQL injection. Also storing the username in the cookie is a bad idea because anyone can forge the cookie to gain authentication.
My answer to the question if this is secure: no.
You need to sanitize your code. What happens if someone enters 'test OR 1=1 ' as username?
I do not really know where to start. This code is really unsafe.
- You should sanitize with
mysql_real_escape_string()
(or mysqli function, or even better: use PDO for any database connection and use prepared statements) the username and the password and be sure that$remember
is either a boolean or an integer. - The
sha1
is something like broken, so i'd suggest usingmd5
instead. - Cookies can be rewritten by the user
that could add
username=admin
to the cookie and login as admin.
Your code is not secure.
Your data is open to SQL injection via the initial query, where depending on the access level of the database user, you could have anyone logging in. You need to sanitise your input.
Secondly, the access to the website via the cookie, and the username in it related to the access level and privilege they get? If so in it's current form the session can be easily hijacked.
Here's A Code I use To Make Sure Everything Is Safe .. It may not be the safest but I also use other measures to verify a safe login. But this code will protect u against SQL injections.
function secure($data) {
$data = trim(htmlentities(strip_tags($data)));
if (get_magic_quotes_gpc())
$data = stripslashes($data);
$data = mysql_real_escape_string($data);
return $data;
}
It's usage
secure($username);
for example
foreach($_POST as $key => $value) {
$get[$key] = secure($value);
}
This tells PHP for each of the POST values secure it. You Can also use it for post by using $_GET instead of $_POST but lets face it .. it would be really stupid to have your login using GET commands
精彩评论