1

I have a form that updates an MD5 encrypted password. At this point in the process the password field has been updated to a random token that was then sent to the user via email as part of a link and then that token is used to match up the account to update.

I have to check two tables because we broke out the admin and users into different tables. I'm getting the following SQL error. SQL and CodeIgniter are both pretty new to me.

SQL Error:

Error Number: 1064

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'UNION UPDATE staff SET staff_password = '098f6bcd4621d373cade4e832627b4f6' WHERE' at line 1

UPDATE admin SET admin_password = '098f6bcd4621d373cade4e832627b4f6' WHERE admin_password = 'fd323219f98afd367fee9907173012db' UNION UPDATE staff SET staff_password = '098f6bcd4621d373cade4e832627b4f6' WHERE staff_password = 'fd323219f98afd367fee9907173012db'

Model Code:

public function update_password() {
    $sql = "UPDATE admin SET admin_password = ? WHERE admin_password = ? UNION UPDATE staff SET staff_password = ? WHERE staff_password = ?";
    $password = $this->input->post('password');
    $token = $this->input->post('token');
    $query = $this->db->query($sql, array($password, $token, $password, $token));

    if($query->num_rows() == 1) {
        return true;
    } else {
        return false;
    }
}

The token is set as a hidden field in the view:

<input type="hidden" name="token" value="<?php echo $token;?>" id="token">
Rich Coy
  • 545
  • 1
  • 8
  • 24
  • 1
    FYI: md5 is *not* a secure way to encrypt passwords. Hasn't been for a long time now: http://stackoverflow.com/a/770923/183254 http://en.wikipedia.org/wiki/MD5 – stormdrain Nov 16 '12 at 20:31
  • Thanks. I am aware of that and we will move to more secure protection before going live. Now it's a matter of getting the site framework built. – Rich Coy Nov 16 '12 at 20:33

2 Answers2

3

You can update the tables individually:

$this->db->update('admin',array('admin_password'=>'098f6bcd4621d373cade4e832627b4f6'),'id = 123');
$this->db->update('staff',array('staff_password'=>'098f6bcd4621d373cade4e832627b4f6'),'id = 456');

You could also use method chaining:

$this->db->where('id','123')->update('admin',array('admin_password'=>'newpass'));

As tadman said, much better to check on the users ID rather than the password.

Check out https://www.codeigniter.com/userguide2/database/active_record.html

And for future reference: https://www.codeigniter.com/userguide2/libraries/encryption.html

Stack Programmer
  • 679
  • 6
  • 18
stormdrain
  • 7,915
  • 4
  • 37
  • 76
2

When updating two tables you need to specify how they are joined:

UPDATE admin, staff
  SET admin_password=? staff_password=?
  WHERE admin_password=? AND staff_password = ?

Usually you'd see something like staff.admin_id=admin.id to ensure the two tables are joined together. What you're doing here looks terrifying. What if two people have the same password? You're going to change them both.

It's a lot safer to update with WHERE admin.id=? to zero in on a particular record.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • I agree this looks a little iffy. I'll look at adding another unique identifier to the url that they click on in the email. – Rich Coy Nov 16 '12 at 20:55
  • Yikes, if you're sending this out by email, create a **random token** that can't be guessed to resolve which user you're supposed to update. Referencing by ID is an exceptionally bad idea. – tadman Nov 16 '12 at 21:05
  • I was using the random token that was placed in the email field to key off of which user to update. – Rich Coy Nov 16 '12 at 21:15
  • So long as it's not the raw User ID being exposed you should be okay. – tadman Nov 17 '12 at 02:19