Safe PHP for login and registration. (resource)

[Lumi]

Update 2017: This is extremely out dated, modify it to use PDO prepared statements.

As I look at the multiple threads in here as of lately with people asking why their registration and login isn't working etc I notice how vulnerable and sloppy their code is so here's some (procedural) php i've written to help people out.

 

Your config:

Usually you want to just store your database link in one file so you can link it in multiple files.

<?php

    /* order as follows db address (usually localhost unless using external), db username (don't use root pls), db user password, db name */

    $db = mysqli_connect("", "", "", "");

 

Login:

<?php
  include 'config.php';

 // include our config.

  session_start(); //start your session. *Name your sessions so they don't conflict with other sites
   $email = (isset($_POST['email']) ? htmlspecialchars($_POST['email']) : ""); //ternary operator to get post data
   $password = (isset($_POST['password']) ? htmlspecialchars($_POST['password']) : ""); //ternary operator to get post data
  
function handleLogin($email, $password, $db)
 {
        $email = strtolower(mysqli_real_escape_string($db, $email)); //str to lower so emails / usernames aren't case sensitive
      $password = hash('sha512', mysqli_real_escape_string($db, $password)); //mysqli real escape string isn't needed on a hash but yolo
      $query = mysqli_query($db, "SELECT * FROM `users` WHERE email='$email'"); //since this is a function and we're escaping the string above it's not vuln
      $row = mysqli_fetch_assoc($query); //i use fetch assoc instead of array, up to your personal preference.
     
 if ($password == $row['password']): //is the password submitted equal to the password stored in the db?
         $ip = @$_SERVER['HTTP_X_FORWARDED_FOR']; //assign to variable because of encapsulated quotes are annoying. 
         $updateip = mysqli_query($db, "UPDATE `users` SET ip='$ip' WHERE email='$email'"); //update ip on login
         $_SESSION['email'] = $email; //set session email
       else:
       //invalid username / password combination
       
  endif;
    
  }

   if(@isset($_SESSION['email']))
  {
       

  // simple way to check if user is already logged in

     
  }
 elseif ($email && $password) //no need for isset, if the vars don't contain anything it won't run whats in the

Register:

 

<?php
include 'config.php';

// include our config for our DB connection

session_start();

// start your session. *Name your sessions so they don't conflict with other sites/* ternary operators to get post data bellow */

$name = (isset($_POST['name']) ? htmlspecialchars($_POST['name']) : "");
$email = (isset($_POST['email']) ? htmlspecialchars($_POST['email']) : "");
$password = (isset($_POST['password']) ? htmlspecialchars($_POST['password']) : "");
$passwordverif = (isset($_POST['passwordverif']) ? htmlspecialchars($_POST['passwordverif']) : "");

function handleRegister($name, $email, $password, $db)
 {
 global $return; //we'll use this later to check info, not the best way but it works
 $name = mysqli_real_escape_string($db, $name); //lets clean this stuff
 $email = strtolower(mysqli_real_escape_string($db, $email)); //lets clean this stuff
 $password = hash('sha512', mysqli_real_escape_string($db, $password)); //again escaping a hash isn't needed but yolo
 $check = mysqli_query($db, "SELECT * FROM `users` WHERE email='$email'");
 $rows = mysqli_num_rows($check);
 if ($rows > 0)
  {
  $return = 'already'; //if a result is returned they're already registered
  }
   else
  {
  $ip = @$_SERVER['HTTP_X_FORWARDED_FOR']; //encapsulated quotes are annoying
  mysqli_query($db, "INSERT INTO `users` (name, email, password, ip) VALUES ('$name', '$email', '$password', '$ip')");
  $_SESSION['email'] = $email; //log them in afterwards
  $return = "registered"; //assign the return global to registered to check down later
  }
 }

if (@isset($_SESSION['email']))
 { //a logged in user cannot access this
 }
elseif ($email && $password && $name) //no need for isset again{
if ($password == $passwordverif) //check if both of their passwords are the same

 {
 handleRegister($name, $email, $password, $db);
 if ($return == 'already'): //there is a user registered with that email already
  elseif ($return == 'registered'): //waow keed we've registered
   else:

    // impossible

   endif;
   }
    else
   { //passwords don't match up (using 2 inputs so they don't typo their pass and need a reset)
   }
  }
 

I know this isn't the best code what so ever but it will point people in the right direction.

Also people who will point out i didn't include salting to the password hash, yolo, it isn't really necessary because of the way hashing works. It just makes the hash a bit more secure. In my opinion it'd be better to worry about your site not being vulnerable in the first place.

 

If you think i missed anything let me know, people do make mistakes.

[SSL]

Also people who will point out i didn't include salting to the password hash, yolo, it isn't really necessary because of the way hashing works. It just makes the hash a bit more secure. In my opinion it'd be better to worry about your site not being vulnerable in the first place lol.

 

This is false. Salting is necessary unless you want to pretend rainbow tables don't exist. Weak password security IS a vulnerability.

 

I'm also confused about the use of a global variable here. Not only is it undesirable to use globals, there isn't even a good reason for it in this case.

 

Finally, best practice these days is to use PDO and prepared statements. Using mysqli_real_escape_string in 2015 should be a red flag.

[Lumi]

Finally, best practice these days is to use PDO and prepared statements. Using mysqli_real_escape_string in 2015 should be a red flag.

I would really like to know how you came to this conclusion.

[Lumi]

I'm also confused about the use of a global variable here. Not only is it undesirable to use globals, there isn't even a good reason for it in this case.

 

"//we'll use this later to check info, not the best way but it works"

It's not a good way, but it works. I took this from an old script I wrote like a year ago.

[ZeferiniX]

Using mysqli_real_escape_string in 2015 should be a red flag.

 

Can you elaborate? I only know if for passwords, it's a red flag but for 2015 in general, why should everyone give it a red flag?

[Lumi]

This is false. Salting is necessary unless you want to pretend rainbow tables don't exist. Weak password security IS a vulnerability.

Yes, php's RNG is also vulnerable because it's not truly random, and guess what most people use to generate a random salt?

[SSL]

I would really like to know how you came to this conclusion.

 

Can you elaborate? I only know if for passwords, it's a red flag but for 2015 in general, why should everyone give it a red flag?

 

Because it's standard best practice to abstract the database and use bound parameters for query data; and manually escaping SQL data like that is dumb when there are better methods.

 

 

Yes, php's RNG is also vulnerable because it's not truly random, and guess what most people use to generate a random salt?

 

That's why it is also a bad idea to use PHP's random number generator.

[fizzlesticks]

php's RNG is also vulnerable because it's not truly random

No RNGs are truly random.

[Lumi]

 and manually escaping SQL data like that is dumb when there are better methods.

I smell opinions!

[Lumi]

No RNGs are truly random.

No, they aren't.

[SSL]

I smell opinions!

 

The argument is that escaping input data is an extra step. Extra code is less efficient, harder to maintain and more prone to errors. In particular it makes an application more vulnerable to second order SQL injection attacks.

 

Prepared statements avoid embedding user-submitted data in queries. Even better would be to use some kind of object-relational mapping library that allows you to avoid writing SQL at all.

[Lumi]

The argument is that escaping input data is an extra step. Extra code is less efficient, harder to maintain and more prone to errors. In particular it makes an application more vulnerable to second order SQL injection attacks.

 

Prepared statements avoid embedding user-submitted data in queries. Even better would be to use some kind of object-relational mapping library that allows you to avoid writing SQL at all.

I agree with you that PDO is a better choice than mysqli but it's not necessary for safe code.

[SSL]

I agree with you that PDO is a better choice than mysqli but it's not necessary for safe code.

 

I didn't argue that PDO is absolutely necessary for safe code, I argued that it is safer, among other advantages. If you'd like to field any kind of argument for using mysqli instead, I'm happy to hear it.

[Lumi]

I didn't argue that PDO is absolutely necessary for safe code, I argued that it is safer, among other advantages. If you'd like to field any kind of argument for using mysqli instead, I'm happy to hear it.

imo its up to personal preference.

[orange]

I didn't argue that PDO is absolutely necessary for safe code, I argued that it is safer, among other advantages. If you'd like to field any kind of argument for using mysqli instead, I'm happy to hear it.

MySQLi is as safe as PDO, as long you are using it correctly, just like PDO, it works best with prepared statements.

MySQLi ≠ MySQL extension, I think you are mistaken.

[SSL]

MySQLi is as safe as PDO, as long you are using it correctly, just like PDO, it works best with prepared statements.

MySQLi ≠ MySQL extension, I think you are mistaken.

 

I'm talking about manual escaping. Honestly who cares, no one here is going to even be creating an application that will be run in a real world situation.

[vorticalbox]

No one here is going to even be creating an application that will be run in a real world situation.

 

I create real world applications as I'm sure a number of other users do on a daily basis. 

[Lumi]

I create real world applications as I'm sure a number of other users do on a daily basis. 

I do as well.

[cubercaleb]

Yes, php's RNG is also vulnerable because it's not truly random, and guess what most people use to generate a random salt?

Aren't all RNG not random? You can't make a circuit board do something random. Of course you could take the system time in nanoseconds and put that in an algorithm to get a number, but still.

[Lumi]

Aren't all RNG not random? You can't make a circuit board do something random. Of course you could take the system time in nanoseconds and put that in an algorithm to get a number, but still.

Apparently random.org is truly random as it does not rely on system time etc.

[fizzlesticks]

Apparently random.org is truly random as it does not rely on system time etc.

It's not, it generates its numbers using atmospheric noise which isn't random. Though it is much better than normal algorithms that come built into languages.

[Lumi]

It's not, it generates its numbers using atmospheric noise which isn't random. Though it is much better than normal algorithms that come built into languages.

You can predict numbers from regular RNG's, I doubt you're going to be able to predict atmospheric noise lol.

[fizzlesticks]

You can predict numbers from regular RNG's, I doubt you're going to be able to predict atmospheric noise lol.

Humans or even computers might not be able capable of doing it but that doesn't mean it's impossible.

In this case it's much easier to imagine sticking a radio in front of one of the sensors, you still wouldn't know what numbers would pop up but you would change what the RNGs come up with. And since you can change the outcome that means it isn't truly random.