How to prevent SQL injection in my code

[GameArea.cz]

Hello, I have just standard html form with username input and password input and it posting to login.php which is like:

$pripojeni = mysqli_connect($DATABASE_HOST, $DATABASE_USER, $DATABASE_PASS, $DATABASE_NAME);
if ( mysqli_connect_errno() ) {
	exit('Nepovedlo se připojit k mySQL databázi: ' . mysqli_connect_error());
}

$jmeno = $_POST['jmeno'];
$heslo = $_POST['heslo'];

$sql = mysqli_query($pripojeni, "SELECT jmeno, heslo FROM databaze WHERE jmeno = '$jmeno'");
while ($data = mysqli_fetch_assoc($sql)){
	$db_jmeno = $data['jmeno'];
	$db_heslo = $data['heslo'];
}

if ($jmeno == $db_jmeno AND $heslo == password_verify($heslo, $db_heslo)){
	$_SESSION['loggedin'] = TRUE;
	$_SESSION['jmeno'] = $jmeno;

And I just got a read my data from my DB by some hacker. I guess via SQL injection. Can you help me what is wront with my code please?

[phdr]

The simplest, fool-proof way, for all data types, of preventing sql special characters in a value from being able to break the sql query syntax, which is how sql injection is accomplished, is to use a prepared query. Because the mysqli extension is overly complicated and inconsistent, especially when dealing with prepared queries, this would be a good time to switch to the much simpler and more modern PDO extension.

As to the posted code -

1. Don’t handle database errors in your code for non-recoverable errors. The only errors that are recoverable by the user are when inserting/updating duplicate or out of range values. This is the only time you should have error handling logic in your code. In fact, in the latest php versions, both the mysqli and PDO extensions always use exceptions for errors and the error handling logic you have now won’t ever be executed upon an error and should be removed.
2. Your post method form processing code should detect if a form has been submitted before referencing any of the form data.
3. You should keep the form data as a set, in a php array variable, then operate on elements in this array variable throughout the rest of the code, i.e. don’t write out lines of code copying variables to other variables for nothing.
4. You should trim all input data, mainly so that you can detect if all white-space characters were entered.
5. You should validate data before using it, storing validation errors in an array using the field name as the main array index.
6. After the end of the validation logic, if there are no errors, use the submitted data.
7. Don’t use a loop to fetch what will be at most one row of data. Just fetch the row and test if there was a fetched row.
8. Again, don’t copy variables to other variables for nothing. Just use the original variables the fetched data is in.
9. If the query matched a row, you know the WHERE … term was TRUE. There’s no good reason to test in your program logic if $jmeno == $db_jmeno. In fact, in your current code, if $jmeno is empty, which shouldn’t match any row, that part of the comparison will be TRUE (an empty value is equal to an empty value.)
10. The logic testing if $heslo == password_verify($heslo, $db_heslo) is not technically correct. Password_verify returns either a true or false value. Comparing this with the value in $heslo doesn’t make sense (it does work, because the value in $heslo is a true value, which will match a true value from password_verify.)
11. You should only store the user id (auto-increment primary index) in a session variable. You should query on each page request to get any other user data, such as the username, permissions, …

Edit: also, in the current program logic, without validation on the server-side and without testing if a row of data was matched, the if ($jmeno == $db_jmeno AND $heslo == password_verify($heslo, $db_heslo)){ logic, for both an empty $jmeno and empty $heslo value, will result in a true result, meaning that $_SESSION[‘loggedin’] will be set to a TRUE value. If this is what you are testing on your secure pages, someone can simply submit empty values to your login code, become logged in, and then can access any of the secured pages.

[GameArea.cz]

I changed the code to this:

$priprava = $pdo->prepare(“SELECT jmeno, heslo FROM databaze WHERE jmeno = ?”);
$priprava->execute([$_POST[‘jmeno’]]);
$uzivatel = $priprava->fetch();

if ($uzivatel && password_verify($_POST[‘heslo’], $uzivatel[‘heslo’]))
{
echo “OK!”;
} else {
echo “NOK!”;
}

what do you think?

[Strider64]

I have found a great way to help people and it’s called chatgpt https://openai.com/blog/chatgpt. It’s not perfect but it can help a person out and I have even used it.

For example I asked how to write a login using PHP and it came up with the following →

// Retrieve the submitted form data
$username = $_POST['username'];
$password = $_POST['password'];

// Validate the form data
if (empty($username) || empty($password)) {
    // Display an error message and redirect the user back to the login form
    $_SESSION['error'] = "Please enter your username and password.";
    header("Location: login.php");
    exit();
}

// Query the database to verify the username and password
$sql = "SELECT * FROM users WHERE username = ? LIMIT 1";
$stmt = $pdo->prepare($sql);
$stmt->execute([$username]);
$user = $stmt->fetch();

if ($user && password_verify($password, $user['password'])) {
    // Set session variables to indicate that the user is logged in
    $_SESSION['user_id'] = $user['id'];
    $_SESSION['username'] = $user['username']; // I personally would not include it
    $_SESSION['logged_in'] = true; /// This is ok, but you already have session user_id

    // Redirect the user to the dashboard or home page
    header("Location: dashboard.php");
    exit();
} else {
    // Display an error message and redirect the user back to the login form
    $_SESSION['error'] = "Incorrect username or password.";
    header("Location: login.php");
    exit();
}

Like I said it isn’t perfect, but I think it can help out people who are new to coding.