My function vs Mysql_real_escape_string

Question

I have sessions that for the website and this is how i use them:

   $username = CleanMe($_SESSION["username"]);
   $password = CleanMe($_SESSION["password"]);

   //return clean values
   $_SESSION["username"] = $username;
   $_SESSION["password"] = $password;

CleanMe is:

       function CleanMe($strWords){ 
       $bad_string = array("select", "drop", ";", "--", "insert","delete", 
       "xp_", "%20union%20", "/*", "*/union/*", "+union+", "load_file", 
       "outfile", "document.cookie", "onmouse", "<script", "<iframe", "<applet", 
       "<meta", "<style", "<form", "<img", "<body", "<link", "_GLOBALS", "_REQUEST", 
       "_GET", "_POST", "include_path", "prefix", "http://", "https://", 
       "ftp://", "smb://", "'", "\""); 
       for ($i = 0; $i < count($bad_string); $i++){ 
       $strWords = str_replace ($bad_string[$i], "", $strWords); 
       } 
       return $strWords; 
       }

Now, does it make sense for me to use mysql_real_escape_string or what i have, CleanMe is more secure?

Where are these values coming from? Are they being used in DB queries? If they are being used in DB queries, use `mysql_real_escape_string`, don't re-invent the wheel. — gen_Eric, Apr 02 '12 at 19:37
For starters, your `CleanMe()` function should probably be using [`str_ireplace()`](http://php.net/manual/en/function.str-ireplace.php) for case-insensitivity; although I would recommend it's probably better to not reinvent the wheel and just use `mysql_real_escape_string()`. — Matthemattics, Apr 02 '12 at 19:38
If you want to be 100% secure, since this is just for usernames and passwords, try using [prepared queries](http://www.ultramegatech.com/2009/07/using-mysql-prepared-statements-in-php/) — Matthemattics, Apr 02 '12 at 19:40
So if the user's password was "dropselect" it would go into your database as ""? That doesn't seem like a very good idea to me. `mysql_real_escape_string()` was written for many reasons, and one of those reasons so functions like this didn't get written. — WWW, Apr 02 '12 at 19:40
@crontab thanks for answering. the only concern i have is that i believe that the mysql_real_escape_string alone won't prevent other attacks... so i am not sure how to go about it. — AAA, Apr 02 '12 at 19:42
If you feel the need to make a list, remember that whitelisting is always better than blacklisting. — Tom, Apr 02 '12 at 20:02

score 3 · Accepted Answer · answered Apr 02 '12 at 19:43

3

Just reuse existing functions as much as possible; mysql_real_escape_string in this case. It's pretty likely that you forgot something in CleanMe which makes it insecure. You only need to forget one string to make it insecure and you may not know what that string is now.

Just remember: an attacker has enough time and has to get it right only once but a developer needs to be right every time. So the lesson is: don't make it harder for yourself and use and apply existing functions properly.

answered Apr 02 '12 at 19:43

Simeon Visser

118,920
18
185
180

Thanks Simeon. I have often been told that simply using mysql_real_escape_string won't cut it. So i am not sure but do you think using mysql_real_escape_string(); would do it? I am using htmlspecialchars to prevent xss attacks. – AAA Apr 02 '12 at 19:44
1

These are two different types of attacks: inserting a malicious query into the database (`mysql_real_escape_string` protects against that) and outputting script code which was not intended (`htmlspecialchars` protects against that). So the short answer is that you need both: one to protect the database and the other to protect code from being run on the machines of visitors of your website. – Simeon Visser Apr 02 '12 at 19:48
ok so while inserting in the database i use mysql_real_escape_string(); and to display it use htmlspecialchars(); ? – AAA Apr 02 '12 at 19:51
Yes. You can use `htmlspecialchars` when displaying content that the user has entered (such as comments, posts or other data that comes from a user input field). And before putting it into your database, you need to use `mysql_real_escape_string`. – Simeon Visser Apr 02 '12 at 19:56
1

See my comment to that answer below. – Simeon Visser Apr 07 '12 at 16:19
Also, can you please check out this question i posted: http://stackoverflow.com/questions/10055924/are-these-steps-enough-to-secure-a-site/10056115#10056115 – AAA Apr 07 '12 at 16:25

score 1 · Answer 2 · edited May 23 '17 at 10:08

Despite of the fact that your Cleanme() function makes absolutely no sense, mysql_real_escape_string() function doesn't make it either (in this context).

You shouldn't use either of them!

for the purpose of whatever "cleaning".

mysql_real_escape_string() is not intended to "clean" anything

but merely to format strings.

Whenever you need this function, you need it despite of "cleaning", but just because it is required by SQL syntax. And where you don't need it, escaping won't help you even a bit.

The usage of this function is simple: when you have to use a quoted string in the query, you have to escape it's contents. Not because of some imaginary injections, but merely to escape these quotes that were used to delimit a string. This is extremely simple rule, yet extremely mistaken by PHP folks.

This is just syntax related function, not security related.

Depending on this function in security matters, believing that it will "clean" your data WILL lead you to injection eventually.

Protecting your queries from injections is a more complex task. See this complete explanation for the full details.

Prepared statements is not a silver bullet too. It covers your back for only half of possible cases. See the important addition I made to the famous question for the details

Stating in a big font that `mysql_real_escape_string` should not be used is making things a lot more confusing for people new to these topics. `mysql_real_escape_string` *should be used* and yes, to be precise, it won't 'clean' the string, it will just escape quotes. `mysql_real_escape_string` won't remove things from your string (which is good commentary) but saying that it should not be used is plain wrong. — Simeon Visser, Apr 07 '12 at 16:17

score 0 · Answer 3 · answered Apr 02 '12 at 19:46

0

You can use mysql_real_escape_string, prepared-statements or go with another abstraction layer. This seems much more secure then CleanMe.

answered Apr 02 '12 at 19:46

tonymarschall

3,862
3
29
52

My function vs Mysql_real_escape_string

3 Answers3

You shouldn't use either of them!