Bigint Not Updating to Current Timestamp

[shawntc]

I'm toying around with PHP and MySQL. A piece of code I have is intended to fetch the current Unix timestamp and update a BIGINT with that value. Problem is, the BIGINT won't update unless there's some echo, var_dump, or other similar statement. Is there something I'm not doing right? (Yes I read that mysql is going to be deprecated... a few minutes ago. This is more of a proof-of-concept script, sharpening my skills.)

//Checks if there is a login cookie
if(isset($_COOKIE['ID_my_site'])){
  //if there is, it logs you in and directs you to the members page
  $username = $_COOKIE['ID_my_site']; 
  $pass = $_COOKIE['Key_my_site'];
  $check = mysql_query("SELECT * FROM `wop_Player` WHERE `Username` = '$username'")or die(mysql_error());
  while($info = mysql_fetch_array( $check )){
    if ($pass != $info['Password']){
    }else{
      header("Location: index.php");
    }
  }
}

//if the login form is submitted 
if (isset($_POST['submit'])) { // if form has been submitted
  // makes sure they filled it in
  if(!$_POST['username'] | !$_POST['pass']) {
    die('You did not fill in a required field. Go back and try again.');
  }

  // checks it against the database
  if (!get_magic_quotes_gpc()) {
    $_POST['email'] = addslashes($_POST['email']);
  }
  $check = mysql_query("SELECT * FROM `wop_Player` WHERE `Username` = '".$_POST['username']."'")or die(mysql_error());

  //Gives error if user dosen't exist
  $check2 = mysql_num_rows($check);
  if ($check2 == 0) {
    die('That user does not exist in our database. <a href="register.php">Click Here to Register.</a>');
  }
  while($info = mysql_fetch_array( $check )){
    $_POST['pass'] = stripslashes($_POST['pass']);
    $info['Password'] = stripslashes($info['Password']);
    $_POST['pass'] = md5($_POST['pass']);
  
    //gives error if the password is wrong
    if ($_POST['pass'] != $info['Password']) {
      die('Incorrect password, please try again.');
    }else{ 
      
      // if login is ok then we add a cookie 
      $_POST['username'] = stripslashes($_POST['username']); 
      $hour = time() + 3600; 
      setcookie(ID_my_site, $_POST['username'], $hour); 
      setcookie(Key_my_site, $_POST['pass'], $hour);      

           // Now update the last login timestamp. (THIS IS THE PROBLEMATIC CODE)
           $now=time();
           mysql_query("UPDATE `wop_Player` SET `LastLogin`=".$now." WHERE `Username`='".$username."'",$con); 

      //then redirect them to the members area 
      header("Location: index.php"); 
    }

 

[misson]

I'm toying around with PHP and MySQL. A piece of code I have is intended to fetch the current Unix timestamp and update a BIGINT with that value. Problem is, the BIGINT won't update unless there's some echo, var_dump, or other similar statement.

Is this supposed to be a statement of what you've observed (a symptom), or your guess as to why the column isn't being updated? If the latter, don't guess.

TIMESTAMP is a more appropriate column type.

Is there something I'm not doing right? (Yes I read that mysql is going to be deprecated... a few minutes ago. This is more of a proof-of-concept script, sharpening my skills.)

Using the outdated mysql extension dulls your skills, as you learn bad habits that you must later unlearn. Only perfect practice makes perfect.

//Checks if there is a login cookie
if(isset($_COOKIE['ID_my_site'])){
  //if there is, it logs you in and directs you to the members page
  $username = $_COOKIE['ID_my_site']; 
  $pass = $_COOKIE['Key_my_site'];
  $check = mysql_query("SELECT * FROM `wop_Player` WHERE `Username` = '$username'")or die(mysql_error());

This is trivially easy to crack. Additionally, it's insecure if you're not using HTTPS, as anyone snooping (on a wireless network &c) can grab usernames & password hashes.

The sample code is vulnerable to SQL injection, which is a very serious security risk. To fix this hole, switch from the outdated mysql extension to PDO and use prepared statements. If you need a PDO tutorial, try "Writing MySQL Scripts with PHP and PDO". The site you save may just be your own.

Don't use die when outputting HTML. You'll get invalid HTML.

Outputting database error messages to non-admin users discloses too much information. Instead, log the MySQL error message. For some errors (such as those related to missing or invalid values), output your own error message to the user and what action the user can take to address it. For the rest, inform the user that there was an internal error.

  while($info = mysql_fetch_array( $check )){
    if ($pass != $info['Password']){

Note that you don't need to use a while loop to fetch results. If there's only supposed to be at most one result row, use an if, which will more clearly indicate intent.

  // checks it against the database
  if (!get_magic_quotes_gpc()) {
    $_POST['email'] = addslashes($_POST['email']);
  }

Never rely on magic quotes. The only time code should check if magic quotes is enabled is to undo it.

What you should be doing instead of mucking about with quoting is using PDO and prepared statements.

  while($info = mysql_fetch_array( $check )){
    $_POST['pass'] = stripslashes($_POST['pass']);
    $info['Password'] = stripslashes($info['Password']);

You've added extra unescapes here; the value stored in the database shouldn't be escaped. Think about what escaping is for, and how MySQL processes statements. Also, take a look at "Programmatic Mutualism".

    $_POST['pass'] = md5($_POST['pass']);

MD5 is considered broken by security professionals. No less than Bruce Schneier has written:

But -- come on, people -- no one should be using MD5 anymore.

           // Now update the last login timestamp. (THIS IS THE PROBLEMATIC CODE)
           $now=time();
           mysql_query("UPDATE `wop_Player` SET `LastLogin`=".$now." WHERE `Username`='".$username."'",$con);

The PHP code doesn't need to handle this. Use a TIMESTAMP column, and either set it to update automatically or set it to NOW():

UDPATE `wop_Player` SET `LastLogin`=NOW() WHERE `id`=?

Overall, the code is very messy. It's too highly coupled because it mixes concerns: database access, authentication logic, user account control, and display, to name a few. Each should be handled by a separate module. If you haven't started learning about OOP, now's the time. You don't need to use OOP to separate concerns, but it makes it easier overall as code separation is built in to the OO paradigm under the moniker "encapsulation".

 

[shawntc]

Yep, the echo and var_dump effects are things I observed.