0

I have a very basic login-check page:

$host="localhost"; // Host name 
$username="user"; // Mysql username 
$password="pass"; // Mysql password 
$db_name="db"; // Database name 
$tbl_name="userdata"; // Table name 
$lastLogDate=date("l, m/d/y, h:i:sa");
// Connect to server and select databse.
mysql_connect("$host", "$username", "$password")or die("cannot connect"); 
mysql_select_db("$db_name")or die("cannot select DB");
$logdate=date("m/d/Y");
$logtime=date("H:i:s");
$logip=$_SERVER['REMOTE_ADDR'];
$nol=$_POST['nol'];
// username and password sent from form 
$myusername=$_POST['myusername']; 
$mypassword=$_POST['mypassword']; 
$origpass=$_POST['mypassword'];
$origpass=str_replace("'", '"', $origpass);
$from=$_POST['from'];
if ($from=="forum") {
  $to="/forum";
}
if (isset($_POST['nol'])) {
  $rd=$nol;
}
else {
  $rd="/home";
}
$sql="SELECT * FROM userdata WHERE username collate latin1_general_cs='$myusername'";
$result=mysql_query($sql);
$result=mysql_fetch_array($result);
$password=$result['password'];
$_SESSION['mypassword']=$mypassword;
$salt = 'no';
$salted_password = $mypassword.$salt;
$mypassword = md5($salted_password);
if($mypassword==$password) {
$mypassword=$_POST['mypassword'];
$_SESSION['myusername']=$myusername;
$_SESSION['mypassword']=$_POST['mypassword'];
  $sqlDate="UPDATE userdata SET lastLog='$lastLogDate' WHERE username='$myusername'";
  $resultDate=mysql_query($sqlDate);
   mysql_query("INSERT INTO login_log (date, time, username, password, ip, success) VALUES ('$logdate', '$logtime', '$myusername', '$origpass', '$logip', '1')");

  header("location:$rd");
}
else {
 mysql_query("INSERT INTO login_log (date, time, username, password, ip, success) VALUES ('$logdate', '$logtime', '$myusername', '$origpass', '$logip', '0')");
  header("location:/login?msg=wrongUorP");
}

After submitting the form, the page is just blank. The error_log doesn't have any errors in it. I haven't touched it since yesterday, and it was doing something yesterday. Another problem is that I have a user-only page redirect to /login?nol=/<page>, so for example if the page is http://www.example.com/random/page/text.txt, the redirect would be /login?nol=/random/page/text.txt. The page checks if the user is logged in with the following:

<?php
session_start();
if(!isset($_SESSION['myusername'])){
  header("location:/login?nol=/random/page/text.txt");
}
?>

But whenever the user logs in, the user just gets redirected to /login?nol=/random/page/text.txt even though the $_SESSION['myusername'] (I think) is already set. (refer to code above). I recently updated to PHP 5.3 with cPanel, and I previously had session_register(). Did I change the session variables wrong? Here is my receiving page:

session_start();
if(!isset($_SESSION['myusername'])){
  //header("location:/login?nol=/home"); // commented out to test if it receives session variable
  echo "<h1>hello</h1>"; //the page displayed this, so the script did NOT receive the variable.
}
kzhao14
  • 2,470
  • 14
  • 21
  • Even if you get this working, your script is susceptible to sql injection attacks. You should learn how to sanitize user input to prevent such hacks. Also doing things like `$origpass=str_replace("'", '"', $origpass);` results in the user having a different password than they originally typed in. – skrilled Sep 16 '15 at 23:51
  • It's not susceptible, because it selects the user's correct password, and compares it to the password they entered. If it is though, could you tell me how? Also, I don't know why that replace is there... – kzhao14 Sep 16 '15 at 23:53
  • 1
    What PHP update? You might have a deprecation notice for the MySQL API. In addition you should use PHP's built in password handling functions. – Jay Blanchard Sep 16 '15 at 23:53
  • I replace the `$origpass=str_replace("'",'"', $origpass)` with `$origpass=str_replace("'", "'", $origpass)`. Also, `$origpass` is only used when entering the login attempt into the login log. – kzhao14 Sep 16 '15 at 23:56
  • I updated to PHP 5.3 within cPanel. I checked the error_log, and no PHP errors. Also, I just checked the PHP manual, and `mysql` function I use are not deprecated. http://php.net/manual/en/migration53.deprecated.php – kzhao14 Sep 16 '15 at 23:56
  • http://jayblanchard.net/demystifying_php_pdo.html http://jayblanchard.net/proper_password_hashing_with_PHP.html – Jay Blanchard Sep 16 '15 at 23:58
  • Also, what is wrong with my password hashing, and how could I fix it? – kzhao14 Sep 17 '15 at 00:00
  • 1
    MD5 is not sufficient any more. – Jay Blanchard Sep 17 '15 at 00:01
  • I tried something, and found that the input for nol is always there, no matter if there is a nol on the login form page. – kzhao14 Sep 17 '15 at 00:03
  • Your use of header() is not robust and could explain variable performance. It should be of the form `header('Location: http://www.example.com/login?msg=wrongUorP');` Note the space after the colon and the absolute URL. – Dean Jenkins Sep 17 '15 at 00:14
  • That's not the problem, though, I just tested it by removing the `header("location`'s, and added a few echos to test the variables, and all echo's displayed the correct information. But, on the /home page (where `$_SESSION['myusername']` is checked, it only echoed after `if (!isset($_SESSION['myusername']))`, while nothing was displayed in `echo $_SESSION['myusername']`. – kzhao14 Sep 17 '15 at 00:19
  • when you want to swing back to the password hash issue, ping me. ooops, I see @JayBlanchard has a comment about that. You can find one of mine [here](http://stackoverflow.com/a/32556010) – Drew Sep 17 '15 at 00:21
  • Alright, also, anyone have a solution to my problem as stated above? – kzhao14 Sep 17 '15 at 00:25
  • Start by commenting out blocks of code until the issue appears/disappears. – Jay Blanchard Sep 17 '15 at 00:29
  • Yeah, I did that and found the issue is with the `$_SESSION` variables. Everything works after checking if the passwords are the same, but on the receiving page (/home), it's not receiving the `$_SESSION` variables. – kzhao14 Sep 17 '15 at 00:32
  • 1
    show where in your code you are doing `session_start()` (the up-top chunk). Not the smaller 2nd blue/gray chunk – Drew Sep 17 '15 at 00:44
  • Hey Drew, could you write that as an answer? I realized I forgot to add a `session_start()` on my check-login script. – kzhao14 Sep 17 '15 at 01:06
  • all's well that ends well. Chat later – Drew Sep 17 '15 at 01:10
  • While Drew pointed out one sql injection point in your script, please note that there are at least 2 other places where an injection could happen. I.e. `$sqlDate="UPDATE userdata SET lastLog='$lastLogDate' WHERE username='$myusername'";` and `mysql_query("INSERT INTO login_log (date, time, username, password, ip, success) VALUES ('$logdate', '$logtime', '$myusername', '$origpass', '$logip', '1')");` – skrilled Sep 17 '15 at 01:47

1 Answers1

1

Session

show where in your code you are doing session_start() (the up-top chunk). Not the smaller 2nd blue/gray chunk

Sql Injection

Jay gave a good reference to his fine site on proper database layers to use, and the proper ways to use them. Many more are written about here. I just injected my table with your

SELECT * FROM userdata WHERE username collate latin1_general_cs='$myusername';

For inserts, doing 2nd level injection timebombs would be easy.

Hash

Jay mentioned it too. See his site. I wrote this up. Get off MD5

Community
  • 1
  • 1
Drew
  • 24,851
  • 10
  • 43
  • 78