Having trouble with PHP updating a database

Question

I've connected to a database by using

function db_connect() {
   $result = new mysqli('localhost','root', '******', 'DB');
   if (!$result) {
     throw new Exception('Could not connect to database server');
   } else {
     return $result;
   }
}

But when I go to register a new customer, I get 'Could not register to DB' and that piece of code is

$result = mysqli_query($conn, "insert into user values('".$username."', sha1('".$password."'), '".$email."')");
  if (!$result) {
    throw new Exception('Could not register to DB.');
  }

And I have a user table in my DB. The values in the table user are:

username varchar(100),
email varchar(150),
passwd varchar(17)

You probably making a mistake in your values. Double check if your SQL is correct. You can show us your user table columns. — Felippe Duarte, Jun 07 '16 at 16:38
In your second snippet, it looks like you have `$conn = db_connect();` before this snippet. Is that correct? — Chris Forrence, Jun 07 '16 at 16:42
don't output fixed error messages. they're basically useless. `throw new Exception(mysqli_error($conn))` will TELL you what failed. — Marc B, Jun 07 '16 at 16:43
@ChrisForrence the first snippet which is the `$result = new mysqli` is first — smithster, Jun 07 '16 at 16:45
Please use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. If you're using a PHP version less than 5.5 you can use the `password_hash()` [compatibility pack](https://github.com/ircmaxell/password_compat). Make sure that you [don't escape passwords](http://stackoverflow.com/q/36628418/1011527) or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. — Jay Blanchard, Jun 07 '16 at 16:45
[Little Bobby](http://bobby-tables.com/) says [your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! — Jay Blanchard, Jun 07 '16 at 16:45
@JayBlanchard I've declared it above `$result = mysqli_query($conn` — smithster, Jun 07 '16 at 16:48
You may have declared `$conn` but did you load it by doing `$conn = db_connect()` somewhere — RiggsFolly, Jun 07 '16 at 16:48
**It would have been useful to have shown that in your code snippet** — RiggsFolly, Jun 07 '16 at 16:49

score 4 · Answer 1 · edited May 23 '17 at 10:29

When you have an INSERT statement without columns, the order of columns will be expected to match the table schema.

Based on that, let's re-examine your snippet:

$result = mysqli_query($conn, "insert into user values('".$username."', sha1('".$password."'), '".$email."')");
if (!$result) {
    throw new Exception('Could not register to DB.');
}

According to this, you're inserting a username, then a password, then an email. However, your table schema defines a username first, then an email, then the password.

Based on that, let's use the below snippet instead. Note that this utilizes prepared statements, which prevents SQL injection attacks.

$query = "INSERT INTO user (username, passwd, email) VALUES (?,?,?)";

if ($stmt = mysqli_prepare($conn, $query)) {

    /* bind parameters for markers */
    mysqli_stmt_bind_param($stmt, "sss", $username, sha1($password), $email);

    /* execute query */
    mysqli_stmt_execute($stmt);

    /* close statement */
    mysqli_stmt_close($stmt);
}

An additional problem: your password field in the database is much too short (17 characters). An SHA-128-hashed password is exactly 40 characters, and to use the password_* methods, you'd need at least 60 characters. I'd advise simply setting the password field to 255 characters.

Please also keep Jay Blanchard's comment in mind to use the password_* family of functions. instead of hashing a password with SHA1.

Nice use of a prepared statement! – Jay Blanchard Jun 07 '16 at 17:01 — Jay Blanchard, Jun 07 '16 at 17:01

score 2 · Answer 2 · edited May 23 '17 at 12:18

If you've properly connected you have a syntax error in your query with the password. Here is your current code:

$result = mysqli_query($conn, "insert into user values('".$username."', sha1('".$password."'), '".$email."')");

If you insist on leaving yourself open for potential SQL injection attacks you would change the concatenation to be like this:

$result = mysqli_query($conn, "insert into user values('".$username."','". sha1($password)."', '".$email."')");

This will allow PHP's sha1() function to execute properly. If you wished to use MySQL's built-in sha1() function then the concatenation here would not be used as I have done.

As stated before Little Bobby says your script is at risk for SQL Injection Attacks. Learn about prepared statements for MySQLi. Even escaping the string is not safe!

In addition you may want to consider using PHP's built-in functions to handle password security. If you're using a PHP version less than 5.5 you can use the password_hash() compatibility pack. Make sure that you don't escape passwords or use any other cleansing mechanism on them before hashing. Doing so changes the password and causes unnecessary additional coding.

On one hand, there does appear to be an SHA1 function available within MySQL ([reference](https://dev.mysql.com/doc/refman/5.5/en/encryption-functions.html#function_sha1)). That being said, using the method in this fashion would expose the plain-text password in any database logs, which is certainly dangerous! — Chris Forrence, Jun 07 '16 at 16:58

RiggsFolly · Answer 3 · 2016-06-07T17:19:56.643

This line is definitely one of your problems

$result = mysqli_query($conn, 
           "insert into user values('".$username."', sha1('".$password."'), '".$email."')");

change it to

$result = mysqli_query($conn, 
          "insert into user values('$username', '$email',  '" . sha1($password) "')");

NOTE Change the order to match your schema as well as de-cluttered the string concatenation.

NOTE2: Your password field is too short to hold a complete hashed password. For future proofing I suggest a full varchar(255)

It would be better to use password_hash() though like this

$hashed = password_hash($password);

$result = mysqli_query($conn, 
          "insert into user values('$username', '$hashed', '$email')");

And then to check when user logs in again use

if ( password_verify($_POST['password'], $db_hashed_password) ) {
    echo 'Password OK';
} else {
    echo 'Password failed';
}

Having trouble with PHP updating a database

3 Answers3