Jump to content

Recommended Posts

Hello,

i want to know if this code is ok or do i have sql-injection, session hijacking etc.

thank you very much for your help.

Rafal

<?php
INI_SET('SESSION.USE_ONLY_COOKIES', 1);
SESSION_START();
SESSION_CACHE_EXPIRE(10);
SESSION_REGENERATE_ID();
$uname = "mail@mail.com";
$upassword = "a4ca6e1f044a98a8a72e7b356a134319433f4d98adb3f463202246bddb883712459e66ea985f37cb2e7171165500c341be4effd1f6e4461246e3c61e5767741f";
if (isset($_POST["inp_name"]) && isset($_POST["inp_pwd"]))
{
if ($uname == $_POST["inp_name"] && $upassword == hash('sha512', $_POST["inp_pwd"]))
{
$_SESSION["e64X96ea"] = 1;
}
}
?>
<?php
if ($_SESSION["e64X96ea"] != 1)
{
header ( 'Location:login.php' );
exit;
}
?> 
Edited by rafal
Link to comment
https://forums.phpfreaks.com/topic/292499-is-this-secure-login-script/
Share on other sites

SQL injection will only occur when you interface with the database.  Since you didn't show this scope, we wouldn't know.  Note that SQLinjection is most easily prevented by using PDO prepared statements.  If the data isn't escaped, the "data" desired to be inputted into the DB can change the SQL query to perform some unintended result.

 

I also don't believe your script indicates whether session hijacking can or can't be accomplished.  I suppose if this is your only script and a user has no ability to add JS to the content, you should be okay.

 

You might want to look at password_hash() instead of hash().

 

Also, I don't think using a cryptic key for your session array (i.e. $_SESSION["e64X96ea"]) provides any protection.

Hello NotionCommotion,

i made some changes and created database based login script.

do i have security problems in this code?

this is the code of index.php

 

thanks

Rafal

<?php
SESSION_START();
include("config.php");
$link1 = new mysqli("$hoster", "$nameuser", "$password", "$basedata") or die ("Connection Error!"); error_reporting (0);
$email = $link1->real_escape_string($_POST["inp_email"]);
$pwd = $link1->real_escape_string($_POST["inp_pwd"]);
if($email && $pwd)
{
$chkuser = $link1->query("SELECT email FROM $table2 WHERE email = '$email' ");
$chkuserare = mysqli_num_rows($chkuser);
if ($chkuserare !=0)
{
$chkpwd = $link1->query("SELECT pwd FROM $table2 WHERE email = '$email' ");
$pwddb = mysqli_fetch_assoc($chkpwd);
if (hash('sha512', $pwd) != $pwddb["pwd"])
{
echo "wrong password!";
}
else
{
$_SESSION['username'] = $email;
header ('Location:list.php');
}
}
else
{
echo "user not found!";
}
}
else
{
echo "enter your email and password!";
}
mysqli_close($link1);
?>
<html>
<body>
<form action="index.php" method="post">
<b>Login</b><br><br>
<table width="100%">
<tr><td>
Email:<br><input type="text" name="inp_email" style="width:98%; padding: 4px;"><br>
Password:<br><input type="password" name="inp_pwd" style="width:98%; padding: 4px;"><br>
<br>
<input type="submit" name="submit" value="Login" style="width:100%; padding: 4px;">
</td></tr>
</table>
</form>
</body>
</html>

Hi Rafal,

 

Its been a long time since I haven't used PDO (and you really should check it out), however, it looks like you are okay with sql injection.

 

Most sites inform the user that the username and/or password is invalid as it prevents a bad buy from first knowing they have a valid username and then trying random passwords with it.  Your approach informs them if they have a valid username but invalid password, and is actually more code intensive (not a big deal) as it queries the database twice. Your choice.

 

In regards to session hijacking, you probably want to rely on others for final judgement.

Not telling the user if a particular name exists is easier said than done, and most people get this wrong.

 

For example, what about the “I forgot my password” feature? In this case, the username or e-mail address is the only input. What if the name doesn't exist? Do you just keep quiet and let the user wait forever for their e-mail? Then they'll be pissed and won't come back.

 

It's also possible to derive information from subtle factors like time differences and errors. For example, you don't check the password at all if the name is already wrong. So a wrong username will abort the script immediately, whereas a wrong password will make the script run much longer. That alone tells me whether or not a name exists, even if you don't explicitly say that. It may also be possible to provoke errors and use the feedback to get information. What if I try to register a name which is already registered? If that leads to an error, I'm again able to tell if a user exists.

 

Long story short, it's very difficult if not impossible to hide usernames. You may try it so that it's at least harder for an attacker to get the information. But the actual solution is to not make the usernames secret in the first place. In that sense, you should not use the e-mail address as the name but rather let people choose arbitrary public names.

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.