Security

Hi Olaf,
Just started looking at the code to see if it could be usefull (I'm sure it is), and saw 2 things of which I think can/might be improved.

In the class:
1) function ins_string($value, $type = "") {
$value = (!get_magic_quotes_gpc()) ? addslashes($value) : $value;
Wouldn't it be better to use a db specific escape function (mysql_real_escape_string())? As far as I know, that's the recommended best practice (see f.e. NYPHP.org phundamentals)

2) $this->ins_string(md5($first_password))
As far as I can understand there are stronger hashes available then md5 nowadays, like SHA256, and it is recommended to use those or at least add a random token to the hashed password.

What are your thoughts about this? I could also give you some sources if you would like.

Matthijs

Hello Matthijs,

The function mysql_real_escape_string() is for sure not a bad addition. In the meantime the current situation should be sql-injection proof. Check the php manual too. I used the md5 hash because it's also used in other languages too and this will make it transportable if someone use an old user database. But I think it's a good idea to offer different "hash" options that the user can decide which he want to use.

Thanks for your suggestions.

gr. Olaf

Hi Olaf,

Thanks for taking the time to reply.

About my suggestions. Are you considering adjusting the class for these 2 issues in the next revision?

I have read the phpmanual on addslashes and mysql_real_escape_string functions. That's why I asked this question. From the manual:
" Using mysql_real_escape_string() around each variable prevents SQL Injection. This example demonstrates the "best practice" method for querying a database, independent of the Magic Quotes setting. "

And from NYPHP phundamentals (http://www.nyphp.org/phundamentals/storingretrieving.php) :
" The main reason for using mysql_real_escape_string instead of addslashes is that the former will handle many more characters that require special handling.(See: http://www.mysql.com/doc/en/String_syntax.html.) Addslashes will only escape " (double quote), ' (single quote) \ (backslash) and NUL (the null byte) with a backslash. Mysql_real_escape_string will take into account the character set of the current connection, and escape characters as needed."
...
"All of the database-specific functions (e.g., pg_escape_string) include these kinds of special cases for the particular database, whereas addslashes does not. For simple data it will work, but there is the possibility you may end up in a situation where addslashes alone will fail."

So from what I know a db specific function is a bit more secure.

As for the second issue: indeed, for backwards compatibility/transportability it is usefull to have md5 alone. Your idea to have the option of choosing betweeen different hash options is a good one. That way, you can choose the best solution for your specific situation. I think if one would build an application from scratch and security is an important issue (when not?) one can choose to use the safer option available, like sha-256. If one does not have that possiblilty, for example when passwords are allready stored/hashed a certain way, one can stick with md5.

Please consider this only as suggestions, I'm just trying to think along. Your work is appreciated.

Cheers, Matthijs

Hello,

yes the first one for sure is easy to integrate and will not harm :D

the second one is more tricky, but I will check this soon.

Olaf