3 ms·

Yikes, that's a lot of bad advice for one article. 3 out of 4 methods it recommends for preventing SQL injection should be avoided and it doesn't even mention p

by nimbix 17y ago

Yikes, that's a lot of bad advice for one article. 3 out of 4 methods it recommends for preventing SQL injection should be avoided and it doesn't even mention prepared statements at all.

duskwuff 17y ago
While we're at it: it also fails to understand the difference between addslashes() and mysql_escape_string(). (It's not just a matter of whether you're assigning the result to a variable or not!)
- mildweed 17y ago
  While you're still at it: get rid of the quotes entirely, just htmlentities( ) where appropriate.
themole 17y ago
They should also be suggesting straight up data validation before sticking anything in the db. No reason to let things get that far if you're expecting a number and they give you garbage.
jbm 17y ago
Like you mention, I cannot understand how a problem like this that has been solved gets such a bad solution. For those, like me, who are slaves to mysql for one reason or another, this is what I do. 1) mysql_real_escape_string 2) sprintf I usually mix the two. Here's an example. $format = "UPDATE `Users` SET `user_password` = '%s' WHERE `user_id` = %d"; $query = sprintf($format, mysql_real_escape_string($password), mysql_real_escape_string($user_id) ); Techincally, I don't have to do mysql_real_escape_string on the $user_id variable, since it is not based on user input and sprintf will automatically convert it to 0 if it isn't a number already (and there are no user_ids that equal 0 or 1 in this particular table) (Before someone asks, all the passwords are salted and encrypted using SHA-256. Although, now that I think about it, they are in a binary format, which makes me wonder if I should use %b instead of %s... and since its in binary format already, I really don't need mysql_real_escape_string for the password either. Ugh )
- duskwuff 17y ago
  You should really look into using PDO: http://php.net/pdo http://php.net/pdo It's got real prepared statements (or a reasonable facsimile thereof for databases that don't support them), and it lets you iterate directly over result sets. All sorts of handy stuff.
abhishekdesai 17y ago
ok ok guys, i have updated the post.

by nimbix 17y ago

Yikes, that's a lot of bad advice for one article. 3 out of 4 methods it recommends for preventing SQL injection should be avoided and it doesn't even mention prepared statements at all.

duskwuff 17y ago

While we're at it: it also fails to understand the difference between addslashes() and mysql_escape_string(). (It's not just a matter of whether you're assigning the result to a variable or not!)

mildweed 17y ago
While you're still at it: get rid of the quotes entirely, just htmlentities( ) where appropriate.

themole 17y ago

They should also be suggesting straight up data validation before sticking anything in the db. No reason to let things get that far if you're expecting a number and they give you garbage.

jbm 17y ago

Like you mention, I cannot understand how a problem like this that has been solved gets such a bad solution. For those, like me, who are slaves to mysql for one reason or another, this is what I do. 1) mysql_real_escape_string 2) sprintf I usually mix the two. Here's an example. $format = "UPDATE `Users` SET `user_password` = '%s' WHERE `user_id` = %d"; $query = sprintf($format, mysql_real_escape_string($password), mysql_real_escape_string($user_id) ); Techincally, I don't have to do mysql_real_escape_string on the $user_id variable, since it is not based on user input and sprintf will automatically convert it to 0 if it isn't a number already (and there are no user_ids that equal 0 or 1 in this particular table) (Before someone asks, all the passwords are salted and encrypted using SHA-256. Although, now that I think about it, they are in a binary format, which makes me wonder if I should use %b instead of %s... and since its in binary format already, I really don't need mysql_real_escape_string for the password either. Ugh )

duskwuff 17y ago
You should really look into using PDO: http://php.net/pdo http://php.net/pdo It's got real prepared statements (or a reasonable facsimile thereof for databases that don't support them), and it lets you iterate directly over result sets. All sorts of handy stuff.

abhishekdesai 17y ago

ok ok guys, i have updated the post.