PHP | SQL - mysqli_stmt_prepare fails and connected to the database

Question

I'm trying to execute a parameterized query to update some stuff in the database.

The problem is that it mysqli_stmt_prepare fails. The require is used to connect to the database.

require 'includes/dbInclude.php';
if ($codeQuery > 0){
    $confirmationUsername = $_GET['confirmationUsername'];
    $active = "active";
    $noCode = "";
    $insertSql = "UPDATE users SET accountStatus = ? WHERE username = $confirmationUsername";
    $insertSql2 = "UPDATE users SET confirmationCode = ? WHERE username = $confirmationUsername";
    $statement = mysqli_stmt_init($connection);
    $statement2 = mysqli_stmt_init($connection);
    if (!mysqli_stmt_prepare($statement, $insertSql)){
        header("Location: registerComplete.php?error=sqlError1");
        exit();
    }
    elseif (!mysqli_stmt_prepare($statement2, $insertSql2)){
        header("Location: registerComplete.php?error=sqlError2");
        exit();
    }
    else{
        mysqli_stmt_bind_param($statement, "s", $active);
        mysqli_stmt_execute($statement);
        mysqli_stmt_bind_param($statement2, "s", $noCode);
        mysqli_stmt_execute($statement2);
    }
}

dbInclude.php contains:

<?php

//connection variables
$serverName = "localhost";
$dbUsername = "root";
$dbPassword = "";
$dbName = "ecglive";

//connection
$connection = mysqli_connect($serverName, $dbUsername, $dbPassword, $dbName);

//connection error
if(!$connection){
    die("There was an error connceting to the database: " . mysqli_connect_error());
}

And where I used it works. I alos tried copy that code to this one just to see if there was any problem connecting to the database. It isn't.

It always goes on the first error if, where it says sqlError1 and if I delete it, then it goes to the sqlError2.

Did I make any mistake?

you miss the point of the prepared statement by directly embedding a variable in the sql statement - worse yet, the variable is a GET variable and not, in any way, sanitised — Professor Abronsius, Apr 19 '19 at 18:15
the unescaped and embedded variable should also be in quotes as presumably it is a string — Professor Abronsius, Apr 19 '19 at 18:16
I'm confused. I read about prepared statements and where I read, it was written something like that. What should I do instead then? — , Apr 19 '19 at 18:18
The key here is to **prepare any and all data values** and not just some of them. As a rule of thumb don't put `$` inside your query strings. You should even consider using single quotes to describe queries so you're not tempted to interpolate those strings. — tadman, Apr 19 '19 at 18:37
Note: The [object-oriented interface to `mysqli`](https://www.php.net/manual/en/mysqli.quickstart.connections.php) is significantly less verbose, making code easier to read and audit, and is not easily confused with the obsolete `mysql_query` interface where missing a single `i` can cause trouble. Example: `$db = new mysqli(…)` and `$db->prepare("…")` The procedural interface is largely an artifact from the PHP 4 era when `mysqli` API was introduced and should not be used in new code. — tadman, Apr 19 '19 at 18:38
Note: A lot of problems can be detected and resolved by [enabling exceptions in `mysqli`](https://stackoverflow.com/questions/14578243/turning-query-errors-to-exceptions-in-mysqli) so any mistakes made aren’t easily ignored. Many return values cannot be ignored, you must pay attention to each one. Exceptions don’t require individual checking, they can be caught at a higher level in the code. — tadman, Apr 19 '19 at 18:38

score 1 · Accepted Answer · edited Apr 19 '19 at 18:26

You need to bind the username in addition to the accountstatus to help mitigate SQL injection.

require 'includes/dbInclude.php';

if ($codeQuery > 0){

    $confirmationUsername = $_GET['confirmationUsername'];
    $active = "active";
    $noCode = "";

    $insertSql = "UPDATE users SET accountStatus = ? WHERE username = ?";
    $insertSql2 = "UPDATE users SET confirmationCode = ? WHERE username = ?";

    $statement = mysqli_stmt_init($connection);
    $statement2 = mysqli_stmt_init($connection);


    if (!mysqli_stmt_prepare($statement, $insertSql)){
        exit(header("Location: registerComplete.php?error=sqlError1") );
    } elseif (!mysqli_stmt_prepare($statement2, $insertSql2)){
        exit(header("Location: registerComplete.php?error=sqlError2") );
    } else{

        mysqli_stmt_bind_param($statement, "ss", $active,$confirmationUsername);
        mysqli_stmt_execute($statement);

        mysqli_stmt_bind_param($statement2, "ss", $noCode,$confirmationUsername);
        mysqli_stmt_execute($statement2);
    }
}

The `username` wasn't escaped, so it wasn't valid SQL. The error message that you ignored contained details. — tadman, Apr 19 '19 at 18:48

score 1 · Answer 2 · answered Apr 19 '19 at 18:47

This code uses a very strange style, and one that's far more verbose than necessary. Here's a more minimal form of same:

require 'includes/dbInclude.php';

// Enable exception reporting
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

if ($codeQuery > 0) {
    try {
      // Prepare one query that sets both properties.
      $stmt = $connection->prepare('UPDATE users SET accountStatus=?,confirmationCode=? WHERE username=?');

      // Bind parameters directly form the source, no variables needed.
      $stmt->bind_param('ss', 'active', '', $_GET['confirmationUsername']);

      // Attempt to execute
      $stmt->execute();
    }
    catch (Exception $e) {
      // Error handling here...
      header("Location: registerComplete.php?error=sqlError2");
      exit();
    }
}

You're really not doing a lot here, so there's no reason for that code to be so verbose.

That being said, if this is a registration system for some kind of user access control layer and this isn't an academic project you should stop working on this code before you create a huge mess. Writing your own access control layer is not easy and there are many opportunities to get it severely wrong.

Any modern development framework like Laravel comes with a robust authentication system built-in. This is a solved problem and there's no need for you to try and re-invent the wheel here.

At the absolute least follow recommended security best practices and never store passwords as plain-text or a weak hash like SHA1 or MD5.

Thanks for the link. Also what do you mean about being verbose? In your code you just omitted some code that I needed like the 2nd statements for example. Also can you tell me why do you think is better or really is better to user oop besides the confusion with msql? Besides that, I actually have established a security system, so If I did everything right what's' the problem with making my own authentication system?, which was what I learned and didn't even know about laravel. What are the possible problems if I made everything right and secure? Finally, it is for my last highschool project. — , Apr 19 '19 at 19:20
What does "I actually have established a security system" mean? Usually that's a mistaken assumption. As for the queries, I've combined both your statements into one statement, as you can set two column values with one query. There is no need for two queries, and there's no need for you to "initialize" each statement before preparing it, the `prepare()` function does both of those things in one step. — tadman, Apr 19 '19 at 19:30
If this is just an academic project then I hope you're learning a lot of things, that's the goal here. I'm just expressing caution because deploying your original code could expose you and your users to severe security risks. To learn more about web security, have a look at the [OWASP series](https://www.owasp.org/index.php/Main_Page) of documents on various concerns. — tadman, Apr 19 '19 at 19:31

PHP | SQL - mysqli_stmt_prepare fails and connected to the database

2 Answers2