hey all,
I’m trying to create a secure login for my site but its not working. Here’s the senerio… If there are three users registered to the website John, Sam, Andy and if I’m logged in as John I can still access Sam and Andy’s pages by typing in their URL… Is there any way this can be solved? My code is listed below:
processing the login:
<?php
if($_POST['submit'])
{
session_start();
$username = $_POST['u'];
$password = $_POST['p'];
$connect=mysql_connect("server","user","pass");
$db=mysql_select_db("db");
if(!$username || !$password)
{
header("location: relogin.html");
}
else
{
$check= "SELECT * FROM users WHERE username='".$username."' AND password = '".md5($password)."'";
$result = mysql_query($check) or die ("error: ".mysql_error());
if(mysql_num_rows($result) == 0)
{
echo "The Username and the Password do not match";
exit();
}
else
{
$row=mysql_fetch_row($result);
if (md5($password) == $row[3])
{
$_SESSION['username'] = $username;
$_SESSION['password'] = $password;
header("location: $username/$username.php") or die(mysql_error());
}
else
{
echo "error";
}
}
}
}
?>
and the php at the top of each users pages:
<?php
session_start();
if (!isset($_SESSION["username"]))
{
header("location: login.html");
exit();
}
else
{
if($username = $_SESSION["username"] && $password = $_SESSION["password"])
{
echo "Hi ".$username;
}
}
?>
also in the last echo above the $username is displayed as a number. Could anyone tell me why this happens?
Any help would be appreciated.
Thanks
stow
Hi there.
While this PHP code so far looks fairly insecure (the input isn’t filtered to prevent injection, you’re passing $password around unencrypted a lot…) the problem you’re having with user pages being cross-accessible is an easy one to solve.
All you need to do is determine who’s page is being viewed, then redirect if that’s not the logged in user.
This should do it:
$pageOwner = substr($_SERVER['SCRIPT_NAME'], strrpos($_SERVER['SCRIPT_NAME'], '/')+1, strlen($_SERVER['SCRIPT_NAME'])-(strrpos($_SERVER['SCRIPT_NAME'], '/')+1)-4);
if($_SESSION['username'] != $pageOwner)
{
header("location: naughty_naughty.html");
exit();
}
As for the username = number issue, I’m not really sure on that one. I’ll look closer.
I changed the ='s to =='s but when its =='s there is no output whatsoever
ermm ok… this is what I’ve put now:
session_start();
$pageOwner = substr($_SERVER['SCRIPT_NAME'], strrpos($_SERVER['SCRIPT_NAME'], '/')+1, strlen($_SERVER['SCRIPT_NAME'])-(strrpos($_SERVER['SCRIPT_NAME'], '/')+1)-4);
if (!isset($_SESSION['username']) || ($_SESSION['username'] !== $pageOwner))
{
header("location: ../login.html");
exit();
}
else
{
$username = $_SESSION['username'];
echo "Hi ".$username;
}
Because you dont have an Else to that If.
Yes, which would give $username the value of $_SESSION[‘username’], and always give true as a result, but it doesn’t explain why $username would contain a number instead of the username, unless the session variable contained a number (or maybe because it wasn’t set?).
$_SESSION['username'] = $username;
twitch twitch Surely you meant to sanitize that.
And to answer the original post a little more clearly, for future reference; Your problem was you confused = (set to) and == (equals to) in your if statement.
hey guido,
Thanks for the solutions they worked well… I’m adding a few of my own bits of security too. Do you have any explanation as to why I get a number instead of the actual username on the users page?
<?php
if($_POST['submit'])
{
session_start();
Always put session start at the beginning of the script. If you posted the entire script it will make no difference in this case, but it is good practice to put it at the very beginning.
$check= "SELECT * FROM users WHERE username='".$username."' AND password = '".md5($password)."'";
Never use user input in a query without sanatizing it. In this case, use mysql_real_escape_string() on $username.
$row=mysql_fetch_row($result);
if (md5($password) == $row[3])
{
If the query returned a row, you already know it has the right username and password, since you put that in the query’s WHERE conditions. No need to check again.
$_SESSION['password'] = $password;
Never store the password in a session. There’s no need, and it’s a possible security problem.
So the first script would become:
session_start();
if ($_POST['submit']) {
$username = $_POST['u'];
$password = $_POST['p'];
$connect=mysql_connect("server","user","pass");
$db=mysql_select_db("db");
if (!$username || !$password) {
header("location: relogin.html");
}
else {
$check= "SELECT * FROM users WHERE username='" . mysql_real_escape_string($username) . "' AND password = '" . md5($password) . "'";
$result = mysql_query($check) or die ("error: ".mysql_error());
if (mysql_num_rows($result) == 0) {
echo "The Username and the Password do not match";
exit();
}
else {
$_SESSION['username'] = $username;
header("location: $username/$username.php") or die(mysql_error());
}
}
}
For the check on each user page, you can try Zarin’s code. Don’t forget to add session_start at the beginning.
Hi Zarin,
Thanks for the quick reply and solution. It worked perfectly. Yeah I’ve got to step up the security now.
stow