Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Can using PHP "if" statements on the results of a SELECT query to determine whether to execute an INSERT query cause unintended data overlap?

I'm a student and this is my first time writing a piece of software. It's a web application on a LAMP stack, and as part of that web application, I wrote the following function to interact with the database when creating a new user:

public function CreateUser($username, $password, $email){
  global $DBHandler, $SQLStatement;
  $SQLStatement = $DBHandler->prepare("SELECT id FROM users WHERE username = :username AND verified > 0");
  $SQLStatement->bindParam(':username', $username);
  $SQLStatement->execute();
  $check = $SQLStatement->fetch();
  if ($check['id']){
    return 2;
  }else{
    $SQLStatement = $DBHandler->prepare("SELECT id FROM users WHERE email = :email AND verified > 0");
    $SQLStatement->bindParam(':email', $email);
    $SQLStatement->execute();
    $check = $SQLStatement->fetch();
    if ($check['id']){
      return 3;
    }else{
      /* Edited out code that generates a random verification code, a random salt, and hashes the password. */
      $SQLStatement = $DBHandler->prepare("INSERT INTO users (username, email, passwordhash, salt, verifycode) VALUES (:username, :email, :passwordhash, :salt, :verifycode)");
      $SQLStatement->bindParam(':username', $username);
      $SQLStatement->bindParam(':email', $email);
      $SQLStatement->bindParam(':passwordhash', $passwordhash);
      $SQLStatement->bindParam(':salt', $salt);
      $SQLStatement->bindParam(':verifycode', $verifycode);
      $SQLStatement->execute();
      return 1;
    }
  }
}

$DBHandler is initiated elsewhere as a PHP Data Object.

It follows these basic steps:

  1. Query the database to see if someone has already verified an account with the desired username.
  2. If the username is available, do another query and do the same check for email.
  3. If both the username and the email are available, generate verify code, salt, hash the password, do a third query to write everything to the database, and return 1 (meaning success).

The idea is that usernames and emails aren't reserved until your account is verified (verified = '1'). Elsewhere there's a script that changes your verified column to '1' if you click an emailed verification link.

My first question is this:

Person A is proposing username "Lorem", and person B has an unverified account that also proposes the username "Lorem". Is it possible for the queries to be executed in the following order?

  1. Person A's script determines the username "Lorem" isn't taken by a verified user
  2. Person B's script verifies their account with username "Lorem"
  3. Person A's script creates their unverified account with username "Lorem"

My second question is:

Is it possible to combine the 3 queries in this script into 1 query with the same if/else logic expressed in SQL instead of PHP, and if so, would that improve performance and/or prevent the above scenario from occurring?

like image 334
ch7527 Avatar asked Nov 03 '22 16:11

ch7527


1 Answers

For a more concurrent solution, you could make the insert conditional:

insert  into users 
        (username, email, ...)
select  :username, :email, ...
where   not exists
        (
        select  *
        from    users
        where   verified > 0
                and (username = :username
                     or email = :email)
        )

This is still not 100% safe in MySQL, but should suffice in practice.

Note that the problem of concurrency in user creation is a luxury problem. I'd just set up a basic version like the one you have, and focus on the more interesting aspects of your website!

like image 168
Andomar Avatar answered Nov 13 '22 00:11

Andomar