Search code examples
phpmysqlmysqlixsssql-injection

Potential security issues of this user form


I'm trying to create a secure sign up form, this is just the first few fields there are going to be about another dozen or so more but that's irrelevant. I'm a bit security concious and I am wondering if the following code is sufficiantly protected against XSS and SQL Injections.

  • I'm using htmlentities as I am displaying some of the user input back to the user if any entries are wrong.
  • I believe I've used prepared statements correctly.
  • I've included the start of the form just to show that since I'm posting to self htmlentities are needed.

    if(isset($_POST['submit'])){
    
    $firstname = htmlentities($_POST['firstname'], ENT_QUOTES, 'UTF-8');
    $lastname = htmlentities($_POST['lastname'], ENT_QUOTES, 'UTF-8');
    $email = htmlentities($_POST['email'], ENT_QUOTES, 'UTF-8');
    $emailrepeat = htmlentities($_POST['emailrepeat'], ENT_QUOTES, 'UTF-8');
    $password = htmlentities($_POST['password'], ENT_QUOTES, 'UTF-8');
    $passwordrepeat = htmlentities($_POST['passwordrepeat'], ENT_QUOTES, 'UTF-8');
    
    
    
    if (empty($firstname)) 
    {   
        $fnError = "Please Enter Your First Name";
    }
    if (empty($lastname)) 
    {
        $lnError = "Please Enter Your Last Name";
    }
    
    
    $getEmail = $mysqli->prepare('SELECT * FROM users WHERE email=?');
    $getEmail->bind_param('s', $email);
    $getEmail->execute();
    $getEmail->store_result();
    $countRows = $getEmail->num_rows;
    if ($countRows > 0) 
    {
        $emError = "Email Address Already Exists";
        $countRows = 0;
    }
    
    
    
    else if (empty($email)) 
    {
    $emError = "Please Enter an Email Address";
    }
    else if (!filter_var($email, FILTER_VALIDATE_EMAIL)) 
    {
    $emError = "Invalid Email Address";
    }
    else if ($email != $emailrepeat)
    {
        $emError = "Emails do not match";
    }
    
    if (empty($password)) 
    {
        $pwError = "Please Enter a Password";
    }
    else if (strlen($password)<6)
    {
        $pwError = "Password must be atleast 6 characters";
    }
    else if ($password != $passwordrepeat)
    {
        $pwError = "Emails do not match";
    }
    
    if (filter_var($email, FILTER_VALIDATE_EMAIL)) 
    {
    
        if ($password == $passwordrepeat and !empty($password) and strlen($password)>5)
        {
            $pwhash = password_hash($password, PASSWORD_BCRYPT, array("cost" => 11));
    
            if($stmt = $mysqli->prepare("INSERT INTO users (first_name, last_name, email, password) VALUES (?, ?, ?, ?)"))
            {
                $stmt->bind_param("ssss", $firstname, $lastname, $email, $pwhash);
                $stmt->execute();
                $stmt->close();
            }
    
    
        }
    }
    }
    
    <form id="addnewuser" action="<?php echo htmlentities($_SERVER['PHP_SELF']);    ?>" method="POST"> 
    </form>
    

Solution

  • 1. Kudos for using prepared statements!

    2. Read below

    Overall this looks quite good from a security standpoint but you are most certainly going to run into issues by using htmlentities() everywhere.

    With your logic, if I want to use a password like ThisIsSo<Secu>r then your code is going to convert it to it to ThisIsSo&lt;Secu&gt;r so it will be impossible for me to login to your website unless you are also using htmlentities() on the login page's username/password.

    When saving user data to your database you should always maintain the integrity of what the user typed and implement the proper escaping upon echo'ing the data. The exception to this would be something like the CKEditor; but the editor takes care of the escaping for you.

    To illustrate this better you will want to use htmlentities() like this:

    <div class="welcome-banner">Welcome <?php echo htmlentities($row['firstname']); ?>!</div>
    

    Or like this:

    <h3>Editing Your Profile</h3>
    <label>First Name</label><br>
    <input type="text" name="firstname" value="<?php echo htmlentities($row['firstname']); ?>">