Is this the right way to parameterize query? Is there any other way?

Well i learned how to parameterize queries in php but i just wanted to ask that is it now totally secure from sql injection or any other type of attacks and if it isnt what betternment can i do to secure it even more?

<?php
include 'db.php';
$name = "";
$pass = "";
if(isset($_POST['send'])) {
    $name = $_POST['name'];
    $sql_u = "SELECT * FROM users WHERE username='$name'";
    $res_u = $connection->query($sql_u);
    if (mysqli_num_rows($res_u) > 0) {
        echo "Sorry Username already taken";
    }
    else {
        $password = $_POST['pass'];
        $hpass = password_hash($password, PASSWORD_DEFAULT);
        $query=$connection->prepare("insert into users (username,password) values (?,?)");
        $query->bind_param('ss',$name,$hpass);
        if ($query->execute()) {
            $query->close();
            header('location:index.php');
        } else {
            header('location:not.php');
        }
    }
}

I want to know if their is a even more secure way than only parameterizing queries?

1 answer

  • answered 2019-06-25 14:32 Bill Karwin

    You're using parameters for the INSERT statement, but you skipped using parameters for the SELECT statement. Without parameterizing the SELECT, you still have an SQL injection vulnerability. You need to use parameters in all cases when you combine untrusted content with your SQL.

    Parameters are a good way to prevent SQL injection when combining dynamic content as values in your SQL queries.

    You asked if there were another way, so I will recommend that you use PDO if you're starting out with a new PHP project. It's a little bit easier than Mysqli. In my opinion, there's no reason to use Mysqli unless you're porting a legacy PHP application that had used the deprecated Mysql PHP extension.

    Here's what it would look like using PDO:

    $name = $_POST['name'];
    $sql = "SELECT COUNT(*) FROM users WHERE username = ?";
    $query = $connection->prepare($sql);
    $query->execute([$name]);
    $count = $query->fetchColumn();
    if ($count > 0) {
        echo "Sorry Username already taken";
    }
    else {
        $password = $_POST['pass'];
        $hpass = password_hash($password, PASSWORD_DEFAULT);
        $sql = "insert into users (username, password) values (?, ?)";
        $query = $connection->prepare($sql);
        if ($query->execute([$name, $hpass])) {
            header('location:index.php');
        } else {
            header('location:not.php');
        }
    }
    

    I'm assuming that the PDO connection was made previously, and that it had been enabled with exceptions. If you don't enable exceptions, you should check return values from every prepare() and execute() call to make sure there are no errors.

    The same is true for Mysqli, you can enable exceptions so you don't have to check for errors manually.

    I also show in the example my preference to use SELECT COUNT(*) instead of SELECT *. It's probably a trivial optimization in this case, but if * refers to many columns or there are many rows matching username = $name then the fetch will need to transfer less data from the database.