HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpModerate

SQL injection safety check

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sqlchecksafetyinjection

Problem

I was wondering if my code is safe for sql injection.
This code just checks if the username exists in my db or not.

$username = $_POST['username'];
$stmt = mysqli_stmt_init($con);
$query = "SELECT username FROM users WHERE username = ?" ;
mysqli_stmt_prepare($stmt, $query);
mysqli_stmt_bind_param($stmt, "s", $username);
mysqli_stmt_execute($stmt);
mysqli_stmt_bind_result($stmt, $user['username']);
mysqli_stmt_execute($stmt);
if (mysqli_stmt_fetch($stmt)){
    if ($user['username'] === $username){
    echo $username, ' exists';
    }
}
elseif (!mysqli_stmt_fetch($stmt)){
    echo $username, ' doesn\'t exists';
}

Solution

SQL Injection-wise, this is completely safe. You don't run any risk of SQL Injection.

However, some parts of your code is not optimal:

mysqli_stmt_execute($stmt);
mysqli_stmt_bind_result($stmt, $user['username']);
mysqli_stmt_execute($stmt);


Why are you executing the statement twice?

Now, imagine that you hade your if-else switched so that you wanted to check for non-existing user first:

if (!mysqli_stmt_fetch($stmt)) {
    echo $username, ' doesn\'t exists';
}
elseif (mysqli_stmt_fetch($stmt)) {
    if ($user['username'] === $username) {
        echo $username, ' exists';
    }
}


This code would not work, and you might not be aware of why exactly. You might be just lucky that you did not code it this way from the start.

The issue is that you are calling mysqli_stmt_fetch twice. You should not do that, there can only be a maximum of one result, which means that the second if will always be false.

Your original code should look like this:

if (mysqli_stmt_fetch($stmt)) {
    if ($user['username'] === $username){
        echo $username, ' exists';
    }
}
else {
    echo $username, ' doesn\'t exists';
}


In fact though, if the first if-statement is true, then the inner if will also be true because of your SQL WHERE condition. So your code could be just this:

if (mysqli_stmt_fetch($stmt)) {
    echo $username, ' exists';
}
else {
    echo $username, ' doesn\'t exists';
}

Code Snippets

mysqli_stmt_execute($stmt);
mysqli_stmt_bind_result($stmt, $user['username']);
mysqli_stmt_execute($stmt);
if (!mysqli_stmt_fetch($stmt)) {
    echo $username, ' doesn\'t exists';
}
elseif (mysqli_stmt_fetch($stmt)) {
    if ($user['username'] === $username) {
        echo $username, ' exists';
    }
}
if (mysqli_stmt_fetch($stmt)) {
    if ($user['username'] === $username){
        echo $username, ' exists';
    }
}
else {
    echo $username, ' doesn\'t exists';
}
if (mysqli_stmt_fetch($stmt)) {
    echo $username, ' exists';
}
else {
    echo $username, ' doesn\'t exists';
}

Context

StackExchange Code Review Q#57095, answer score: 10

Revisions (0)

No revisions yet.