Database not updating - php PDO

Question

I'm using the following code to update the password and salt fields in my database :

// First we execute our common code to connection to the database and start the session 
require("common.php"); 

 $id = $_GET[id];

// This if statement checks to determine whether the registration form has been submitted 
// If it has, then the registration code is run, otherwise the form is displayed 
if(!empty($_POST)) 
{  
    // Ensure that the user has entered a non-empty password 
    if(empty($_POST['password'])) 
    { 
        die("Please enter a password."); 
    } 

    // Ensure that the user has entered a non-empty username 
    if(empty($_POST['confirmpassword'])) 
    { 
        // Note that die() is generally a terrible way of handling user errors 
        // like this.  It is much better to display the error with the form 
        // and allow the user to correct their mistake.  However, that is an 
        // exercise for you to implement yourself. 
        die("Please confirm your password."); 
    } 

    if ($_POST["password"] == $_POST["confirmpassword"]) {

        // An INSERT query is used to add new rows to a database table. 
        // Again, we are using special tokens (technically called parameters) to 
        // protect against SQL injection attacks. 
        $query = "UPDATE Staff SET password=:password, salt=:salt WHERE id=:id"; 

        // A salt is randomly generated here to protect again brute force attacks 
        // and rainbow table attacks.  The following statement generates a hex 
        // representation of an 8 byte salt.  Representing this in hex provides 
        // no additional security, but makes it easier for humans to read. 
        $salt = dechex(mt_rand(0, 2147483647)) . dechex(mt_rand(0, 2147483647)); 

        // This hashes the password with the salt so that it can be stored securely 
        // in your database.  The output of this next statement is a 64 byte hex 
        // string representing the 32 byte sha256 hash of the password.  The original 
        // password cannot be recovered from the hash. 
        $password = hash('sha256', $_POST['password'] . $salt); 

        // Next we hash the hash value 65536 more times.  The purpose of this is to 
        // protect against brute force attacks.  Now an attacker must compute the hash 65537 
        // times for each guess they make against a password, whereas if the password 
        // were hashed only once the attacker would have been able to make 65537 different  
        // guesses in the same amount of time instead of only one. 
        for($round = 0; $round < 65536; $round++) 
        { 
            $password = hash('sha256', $password . $salt); 
        }  

        try 
        { 
            // Execute the query to create the user 
            $stmt = $db->prepare($query); 
            $stmt->execute(array(
            ':password' => $password,
            ':salt' => $salt,
            ':id' => $id)); 


        } 
        catch(PDOException $ex) 
        { 
            // Note: On a production website, you should not output $ex->getMessage(). 
            // It may provide an attacker with helpful information about your code.  
            die("Failed to run query: " . $ex->getMessage()); 
        } 

        // This redirects the user back to the login page after they register 
        header("Location: login.php"); 

    }

    die("Passwords do not match.");  
}

There is an 'id' field in the database, and a member of staff with the id equal to 1 (the link on the previous page passes the id to this page, in this example the id would be 1). I'm not sure why it is not updating the database. I'm new to php and would love any help.

Thanks, Joe

Please use a real password hashing algorithm, such as one provided by PHP's `password_hash()` funciton. `sha256` is not suitable for password hashing. — SamT, Sep 23 '13 at 21:35
What @SamT said. This is a ghetto attempt at bcyrpt/PBKDF/similar that falls far short of either. — Sammitch, Sep 23 '13 at 21:37
I'm aware this is a bit old school, but it's just a test. There is no error, it just doesn't update — JoeMorgan, Sep 23 '13 at 21:50
Does your execute function return false? *Returns TRUE on success or FALSE on failure.* — MisterBla, Sep 23 '13 at 21:58
no, it redirects me to the login page as it should, but does not update the database — JoeMorgan, Sep 23 '13 at 22:05
You should always `exit` after sending a `Location` header. Also, can you put some debugging around `$id`? I can only assume this is not being set to the expected value — Phil, Sep 23 '13 at 22:37
Try `echo $db->errorInfo()` after `$db-execute()` and see if we can get some useful information. — Inglis Baderson, Sep 24 '13 at 03:07
I get the following error: Array Warning: Cannot modify header information - headers already sent by (output started at /home/content/47/11368447/html/CCC/changepassword.php:64) in /home/content/47/11368447/html/CCC/changepassword.php on line 75 Redirecting to stafflist.php — JoeMorgan, Sep 24 '13 at 11:21
When I use - $id = '1'; - the database updates successfully. The $_GET['id']; just doesn't work. Any idea why? — JoeMorgan, Sep 24 '13 at 13:08

score 1 · Answer 1 · edited Sep 23 '13 at 22:28

1

Incorrect syntax, you want to call the $id using:

$id = $_GET['id'];

edited Sep 23 '13 at 22:28

Phil

157,677
23
242
245

answered Sep 23 '13 at 21:38

NickW

141
1
6

I've updated the code but it is still not updating the database. Thanks – JoeMorgan Sep 23 '13 at 21:49
1

Shouldn't it be `$id = $_GET['id'];`? – Oriol Sep 23 '13 at 22:22
gah! yes, helps if I don't cock up my own answer! – NickW Sep 23 '13 at 22:23
This will only trigger an `E_NOTICE` level error about an undefined constant (which the OP is probably suppressing via their `error_reporting` level). PHP will still convert `id` to `'id'`, therefore I highly doubt this is the problem. – Phil Sep 23 '13 at 22:28
this does not fix the problem, any other ideas? – JoeMorgan Sep 23 '13 at 22:39
When I use - $id = '1'; - the database updates successfully. The $_GET['id']; just doesn't work. Any idea why? – JoeMorgan Sep 24 '13 at 13:07

Mihai · Answer 2 · 2013-09-23T22:51:09.980

1

I think when you do execute(array)blah it treats all variables as string,so use

http://www.php.net/manual/en/pdostatement.bindparam.php

$stmt ->bindParam(':password', $password, PDO::PARAM_STR)
$stmt ->bindParam(':salt', $salt, PDO::PARAM_STR)
$stmt ->bindParam(':id', $id, PDO::PARAM_INT)
$stmt ->execute();

edited Sep 23 '13 at 22:51

answered Sep 23 '13 at 22:37

Mihai

26,325
7
66
81

Database not updating - php PDO

2 Answers2