rafal Posted November 16, 2014 Share Posted November 16, 2014 (edited) 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 November 16, 2014 by rafal Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted November 16, 2014 Share Posted November 16, 2014 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. Quote Link to comment Share on other sites More sharing options...
rafal Posted November 16, 2014 Author Share Posted November 16, 2014 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> Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted November 16, 2014 Share Posted November 16, 2014 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. Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted November 18, 2014 Share Posted November 18, 2014 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. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.