Is this a bit overboard or am I doing the right thing?

Question

Possible Duplicate:
Best way to prevent SQL injection?

For logging in:

$username = mysql_real_escape_string(htmlspecialchars(strip_tags(trim($_POST['username'])), ENT_QUOTES));
$password = mysql_real_escape_string(htmlspecialchars(strip_tags(trim($_POST['password'])), ENT_QUOTES));

For inserting data I re-use the same mysql_real_escape_string(htmlspecialchars(strip_tags(trim(...

I feel like this is bad practice because I'm using so many functions... Is this the right way to protect against mysql injection & prevent xss injection? Or is it completely overboard? Everything works fine and nothing is broke--my question really is, am I using things that are obsolete when paired together? Is there only one function that I should use for the job?

Thanks.

Don't insert the password into your database as plain text... hash it and salt it... and then you don't need all those fancy htmlspecialchars and striptags for it either — Mark Baker, Apr 29 '11 at 10:20
`mysql_real_escape_string()` is the only necessary call here — Pekka, Apr 29 '11 at 10:23

score 2 · Answer 1 · answered Apr 29 '11 at 10:18

2

mysql_real_escape_string should be enough… the other things you should check where the user registers himself

answered Apr 29 '11 at 10:18

Flask

4,966
1
20
39

score 2 · Accepted Answer · edited Apr 29 '11 at 10:27

2

What if I use <mysecretpassword> as a password?

It will be stripped and anyone will be able to login as me.

I think you should store the username and password as it is and do htmlspecialchars only when displaying them.

strip_tags seems to be unnecessary here at all unless you really dislike usernames like BlaBla aka Yada-Yada <C00lHax0r>

edited Apr 29 '11 at 10:27

Starx

77,474
47
185
261

answered Apr 29 '11 at 10:19

Quassnoi

413,100
91
616
614

I don't dislike those:o I just was using strip_tags to remove html from usernames. Htmlspecialchars should fix that tho right? – Kyle Apr 29 '11 at 10:32

score 1 · Answer 3 · answered Apr 29 '11 at 10:19

You did protect from MySQL injection, but the string you stored is way off from its original format. Functions such as strip_tags, htmlspecialchars and trim should be used when you are pulling the string OUT and echoing it.

Reason is, sometimes you might want to have the string in its original format so you can, for example, just strip some tags, not all of them - or you might want to just use htmlspecialchars without stripping any tags. The key is in being able to easily transform the string in what you need it to be when you are showing it. That means, you need to keep the string in its original format. In order to do that - you don't strip_tags or htmlspecialchars-it.

The other thing is, everyone and their grandparents are using PDO, you might want to start playing with it because it really does make you immune against SQL injection.

What is PDO? I never heard of that till now. I know there's mysqli but I haven't really played with it yet. — Kyle, Apr 29 '11 at 10:22
PDO or Php Data Objects - www.php.net/PDO - it's a lightweight object interface for communicating with various database systems. It offers various functions and the best thing is that it's bundled with PHP and you can extend it as any other class that comes with PHP. — Michael J.V., Apr 29 '11 at 10:26

score 1 · Answer 4 · answered Apr 29 '11 at 10:25

The strip_tags thing isn't necessary.

For coding comfortability, you may want to create a function to to this for you:

function escape_everything($something)
{
    return mysql_real_escape_string(htmlspecialchars(strip_gpc(trim($something)), ENT_QUOTES));
}

function strip_gpc($something)
{
    return get_magic_quotes_gpc() ? stripslashes($something) : $something;
}

This approach has some issues though. It just works for data you are sending to the database, and more important, you are saving data html-encoded. If in the future, you want to generate PDF's from the data you saved in the database, you'd need to htmlspecialchars_decode() first, so it may be a bit inconvenient (but easily solvable).

score 1 · Answer 5 · answered Apr 29 '11 at 10:30

1

Here is what is suggested by most of the answers.

$username = mysql_real_escape_string(trim($_POST['username']));

$password = md5("mysalt".mysql_real_escape_string(trim($_POST['password'])));

answered Apr 29 '11 at 10:30

Starx

77,474
47
185
261

Salt must **not** be a fixed value. It must change with every post. A timestamp would make an excellent salt. something like `$password = md5(date("m-d").trim($_POST['password']));` (No need for escaping the password as it gets hashed beyond recognition anyway). Remember to store that date in the password table, so you can recall it when rehashing the entered password. – Johan Apr 29 '11 at 11:13
@Johan, I think you are completely unaware of this. If the salt is not known to the application in some manner, how will you verify the user the next time he/she logs in? As per your suggestion, the time stamp also needs to be stored while storing the users in the db, otherwise you will not be able to authenticate the user. – Starx Apr 29 '11 at 11:18
sorry but it's you who is completely unaware. The salt should be saved in plaintext along with the password, as @Johan also said and you noted. Usually what one stores in the password field in the database is `$random_salt . hash($random_salt . $user_password)`, with the salt with a known lenght or using a separator between salt and password. – Carlos Campderrós Apr 29 '11 at 13:20
@carlos, My point is, whether the salt is static or dynamic or random. It has to be know to the application in some manner. And I dont remember stating that salt should not be saved in plain text. – Starx Apr 29 '11 at 13:37

Is this a bit overboard or am I doing the right thing?

5 Answers5