0

I'm trying to allow logged in users of my site to be able to change their password which will then be updated in my database. When I click the submit button, I get 'Unknown column '[username]' in 'where clause'. I've tried multiple things and I can't seem to get it to work. I'm a beginner with PHP so don't have an expansive skillset, so I'm not sure what the problem could be. If anyone could help me out I'd appreciate it, thanks.

<?php
session_start();


require_once ("db_connect.php");
require_once($_SERVER['DOCUMENT_ROOT'] . '/functions/functions.php');


$oldpw = ($_POST['oldpw']);
$newpw = ($_POST['newpw']);
$conpw = ($_POST['conpw']);
$currentpw = $_SESSION['password'];

if ($_POST['change'] == 'Change') {
    if ($oldpw && $newpw && $conpw) {
        if ($newpw == $conpw) {
            if ($db_server){
                mysqli_select_db($db_server, $db_database);
                $oldpw = salt($currentpw);
                // check whether username exists 
                $query = "SELECT password FROM users WHERE 'username'= '" . $_SESSION['username'] . "'";
                $result = mysqli_query($db_server, $query);
                if(!$result){
                    $message = "<p class='message'>Error: Coud not connect to the database.</p>" ;
                }else{
                    $newpw = salt($newpw);
                    $query = "UPDATE users SET password = '$newpw' WHERE username = " . $_SESSION['username'] . "";
                    mysqli_query($db_server, $query) or
                            die("Insert failed. " . mysqli_error($db_server));
                    $message = "<p class='message'>Your password has been changed!</p>";
                    // Process further here 
                    mysqli_free_result($result);
                }
            }else{
                    $message = " <p class='message'>Your current password is incorrect.</p>";
            }
        }else{
            $message = "<p class='message'>Your new passwords do not match.</p>";
        }
    }else{
        $message = "<p class='message'>Please fill in all fields.</p>";
    }
}
?>

This is the html I've used:

<form action='change-password.php' method='post' id="register-form">
     <?php echo $message; ?>
         <input class="password-field" type='password' name='oldpw' value='<?php echo $username; ?>' placeholder="Current Password"><br />  
         <input  class="password-field" type='password' name='newpw' placeholder="New Password"><br />
         <input class="password-field" type='password' name='conpw' placeholder="Confrim Password">
         <input class="button" type='submit' name='change' value='Change' />
 </form>
  • To find out the real error, you should use `echo mysqli_error($db_server);` instead of your `Error: could not connect...` custom message. – Michael Berkowski Jan 11 '15 at 16:50
  • I am guessing it is because the username `$_SESSION['username']` is not surrounded in single quotes as a SQL string in that query... – Michael Berkowski Jan 11 '15 at 16:50
  • 1
    Strings have to be in quotes. Also why are you storing the password plaintext? – Charlotte Dunois Jan 11 '15 at 16:50
  • This will be true also of the subsequent `UPDATE` statement for both the username and `$newpw` See [When to use single quotes, double quotes, backticks....](http://stackoverflow.com/questions/11321491/when-to-use-single-quotes-double-quotes-and-backticks) – Michael Berkowski Jan 11 '15 at 16:51
  • @CharlotteDunois There is a `salt()` function in there on the passwords. I would assume that is doing some salt+hash operation... – Michael Berkowski Jan 11 '15 at 16:52
  • @MichaelBerkowski He or she is hashing the current password and then he or she is doing nothing with it. He or she has to hash the new password. – Charlotte Dunois Jan 11 '15 at 16:53
  • @CharlotteDunois It's there: `$newpw = salt($newpw);` right before the `UPDATE` that uses it. – Michael Berkowski Jan 11 '15 at 16:54
  • This would be an excellent time to start learning how to use [`prepare()/bind_param()/execute()`](http://php.net/manual/en/mysqli.prepare.php) in MySQLi, which would avoid the SQL quoting problems while also improving security. There are some examples [here...](http://stackoverflow.com/a/60496/541091) – Michael Berkowski Jan 11 '15 at 16:56
  • I've added the single quotes to $_SESSION['username'] in the update statement and echoed the mysqli_error, now when I submit the form i get 'unknown column '[username]' in 'where clause' – Bethany Gordon Jan 11 '15 at 17:04
  • @BethanyGordon - that means that there's no column called `username` in that table. What is its structure? – andrewsi Jan 11 '15 at 17:06
  • Please also edit above to show the current state of your code. The `unknown column '[username]'` message is suspect, and suggests the wrong thing may have been quoted (the column name rather than the value, for instance) – Michael Berkowski Jan 11 '15 at 17:08
  • there is, I have 4 columns in the table called ID, username, password and datelog – Bethany Gordon Jan 11 '15 at 17:08
  • @BethanyGordon I see you updated the error message above, but can you please also edit to update the SQL statements as you currently have them? – Michael Berkowski Jan 11 '15 at 17:15
  • I've got it working! the working statements read: $query = "SELECT password FROM users WHERE 'username' = '" . $_SESSION['username'] . "'"; and $query = "UPDATE users SET password = '$newpw' WHERE username = '" . $_SESSION['username'] . "'"; – Bethany Gordon Jan 11 '15 at 17:21
  • This can't actually be working as intended: `WHERE 'username' =` That is a column name and should be unquoted ``WHERE username = '" . $_SESSION['username'] . "'....` – Michael Berkowski Jan 11 '15 at 17:25
  • `if (!$result)` successfully checks if the query _failed_ but doesn't check if it didn't return any rows (because username doesn't exist). for that, you would need to check `if (mysqli_num_rows($result) > 0)` to know if you should proceed to do the `UPDATE`. – Michael Berkowski Jan 11 '15 at 17:27
  • It's silly to do this in the comment thread-- I'll type it up as a proper answer below... – Michael Berkowski Jan 11 '15 at 17:28
  • It updated the salted password in the database, but I've unquoted username, and added if (mysqli_num_rows($result) > 0) before $newpw = salt($newpw) and its still works – Bethany Gordon Jan 11 '15 at 17:32
  • @BethanyGordon There are some other things - like verifying that the old password was submitted correctly before update. I added that below. – Michael Berkowski Jan 11 '15 at 17:41

1 Answers1

0

The initial problem you're facing is the fact that the SQL string values for $_SESSION['username'] and $newpass were not properly single-quoted in the SQL strings. For a run-down on when and how to quote inside SQL statements, have a look at When to use single quotes, double quotes, backticks in MySQL.

Turn on error reporting, always when developing code. At the top of your script:

// Disable this when your code is live...
error_reporting(E_ALL);
ini_set('display_errors', 1);

So Using your current code, quoting the strings but not the column names would appear as below.

$oldpw = salt($currentpw);

// check whether username exists 
$query = "SELECT password FROM users WHERE username= '" . $_SESSION['username'] . "' AND password='$oldpw'";
//----------------------------------no quotes^^^^^^^--single-quotes^^^^^^^^^^^^^^^^
// Also adds a check that $oldpass is correct!

$result = mysqli_query($db_server, $query);
if(!$result){
    // This is ambiguous. It should probably show an error related to the query, not connection
    $message = "<p class='message'>Error: Coud not connect to the database.</p>" ;
}else{
    // This should only be done if a row was returned previously
    // Test with mysqli_num_rows()
    if (mysqli_num_rows($result) > 0) {
      $newpw = salt($newpw);

      // Adds single quotes to the username here too...
      $query = "UPDATE users SET password = '$newpw' WHERE username = '" . $_SESSION['username'] . "'";
      mysqli_query($db_server, $query) or
            die("Insert failed. " . mysqli_error($db_server));
      $message = "<p class='message'>Your password has been changed!</p>";
      // Process further here 
      mysqli_free_result($result);
    }
    else {
       // Username or password was incorrect - do something about that
    }
}

This can be further improved using prepared statements. Your hashed password should be safe from SQL injection, but it is highly recommended to get into the habit of using prepared statements, as they are necessary for other cases where strings derive directly from user input to offer sufficient protection against SQL injection.

That would look like:

// Prepare the select statement to check username and old password
$stmt = mysqli_prepare($db_server, "SELECT password FROM users WHERE username = ? AND passowrd = ?");
if ($stmt) {
  // Bind parameters and execute it
  $stmt->bind_param('ss', $_SESSION['username'], $oldpw);
  $stmt->execute();

  // num_rows works here too...
  if ($stmt->num_rows > 0) {
    // Ok to update...
    // Prepare another statement for UPDATE
    $stmt2 = mysqli_prepare($db_server, "UPDATE users SET password = ? WHERE username = ?");
    if ($stmt2) {
       // Bind and execute
       $stmt2->bind_param('ss', $newpass, $_SESSION['username']);
       $stmt->execute();
    }
    // Error updating
    else echo mysqli_error($db_server);
  }
}
// Error selecting
else echo mysqli_error($db_server);
Community
  • 1
  • 1
Michael Berkowski
  • 267,341
  • 46
  • 444
  • 390