patternphpModerate
SQL injection safety check
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.
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:
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:
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
Your original code should look like this:
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:
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.