Question regarding SQL injection

[eluser]Comanche[/eluser]
Hi there,

I'm currently checking my current project for security against sql injection.

I use query bindings most of the time but I'm not sure if I understand the whole thing right.

For example I have the follwing code fragment:

Code:

$query = array( $data['name'],
                $data['passwd'],
                $data['kid']);
              
$sql = "UPDATE customers SET
            name=?,
            passwd=?
        WHERE
            kid=?";
                          
$this->db->query($sql, $query);

All the data is passed via POST to the page and at this point I already checked the values, so I know for sure, that kid will be a positive integer.
My problem is the 'passwd'-field, since I want to allow all characters to make brute force attacks less efficient (makes about 95 possible characters per position, at least 6 characters per password -> 95^6 possible passwords). That means that I check 'passwd' for [:graph:] with preg_match.
But that means also that I can type any possible mysql command into the password field. So one could type

0 WHERE kid=%; #

which would result in this query

Code:

UPDATE customers SET name=xyz, passwd=0 WHERE kid=%; # WHERE kid=abc;

I'm wondering if query bindings really escape everything which means that the injection from above would result in

Code:

UPDATE customers SET name='xyz', passwd='0 WHERE kid=%; #' WHERE kid='abc';

Edit: Ok, this example isn't perfect since I don't allow whitespaces within the password. But the basic problem would remain the same for a password like

0;####

which would result in

Code:

UPDATE customers SET name=xyz, passwd=0;#### WHERE kid=abc;

I'm also wondering what happens to double and single quotes within the password.

[eluser]Pascal Kriete[/eluser]
The function checks the type and surrounds strings with quotes. So it becomes:
system/database/DB_driver.php - line 679

Code:

UPDATE customers SET name='xyz', passwd='0;####' WHERE kid='abc';

Double and single quotes are escaped with backslashes by the native mysql_real_escape_string function (or the appropriate equivalent for your db type).

The bigger issue with your code is that you're storing passwords in plain text. So instead of the brute forcing of each user's account that you're anticipating, the hacker will do it exactly once - to get the database credentials. You should hash your passwords.

[eluser]drewbee[/eluser]
Yup. md5() and salt those passwords!

[eluser]Comanche[/eluser]
Hi,

thanks for your reply.
I know about the problem with the plaintext passwords, but I can't change it since I'm just developing a new administration interface for the whole thing but there are other applications which need plaintext passwords from the database.
In fact the query I posted isn't complete since there also is a second column which stores the crypted password.

I don't know exactly why it has to be that way but until now, even though I mentioned this problem, I wasn't allowed to change the whole system.

It's a little bit more secure since it's just reachable via intranet and access is limited to some ip addresses but in fact you're right, the security level could be (and should be) higher.

Edit:
Even though access is limited I want to develop an interface which can resist attacks like sql injection or brute force (or user input errors).

[eluser]dcunited08[/eluser]
First off, I would hash the password before storing it in the database. That way there is no way for the attacker to SQL inject and get all the password such as:

Code:

//first tic is the injected piece
Select * from customers where name = 'xyz' or 1=1'

or, even worse

Code:

//reset all passwords for all customers
UPDATE customers SET name='xyz', passwd='passForAllAcounts' WHERE 1=1; #' WHERE kid='abc' or 1=1; #';

You would still have to do more to protect the 'kid' and the 'name' inputs but those you are limiting to alphanumeric, I would assume.

Secondly, make sure you remove the possibility of all escape characters by encoding them, take a look at php function [url="http://us2.php.net/htmlspecialchars"]htmlspecialchars[/url];

[eluser]dcunited08[/eluser]
Let me get this straight, there are two columns, one in encrypted (not hashed) and one is not?

[eluser]Comanche[/eluser]
Hi,

I would hash the passwords if it would be possible, but at the moment I have the order to save them in plain text As soon as I'm allowed to do so I would use SHA-256 with a salt to save them (I'm studying 'IT-Security' so it's not the missing knowledge about security features which prevents me from using hashed passwords ).

For the second thing you mention, 'kid' is limited to positive integer by

Code:

function validatePosInteger($num)
{
    return is_numeric($num) && ($num >= 0) && ($num == (int)$num);
}

and 'name' is limited to alphanumeric, '.', '&', '-' and space. There is also a second check if a 'kid' with a given value exists within the database.

Thanks for mentioning 'htmlspecialchars', I wasn't thinking about it until now but it's sure a good thing to do so

[quote author="dcunited08" date="1226094207"]Let me get this straight, there are two columns, one in encrypted (not hashed) and one is not?[/quote]
Yes, and please don't ask me why, the system as it is was introduced nine years ago, at that time I was visiting 10th grade at school

[eluser]dcunited08[/eluser]
In that case, I would suggest limiting your allowed characters, search the database to make sure they are not already in use, and whitelist (regex work great) the allowed characters and give errors if the character is not allowed. Think of it this way, if you allow more secure passwords but also allow SQL Injection the system is more at risk then limiting the security of passwords and removing the vulnerability. You can also use [url="http://us3.php.net/mysql_real_escape_string"]mysql_real_escape_string[/url] since you are using MYSql. (disregard the htmlspecialchars)

[eluser]Rick Jolly[/eluser]
@dcunited08 - If using query bindings or active record, there is no need to manually use mysql_real_escape_string.