0

What's the best way to use mysql_real_escape_string, is it at the beginning like this:

$email = mysql_real_escape_string($_POST['email']); 
$qemail = mysql_query ("SELECT email FROM ppl WHERE email='$email'"); 

or at the end like this:

$email = $_POST['email'];
$qemail = mysql_query ("SELECT email FROM ppl WHERE email='". mysql_real_escape_string($email) ."'");

The whole website is using mysql so I have to keep it in mysql. The problem is, I don't want to use mysql_real_escape_string everywhere (the code looks confusing and horrible). I would like to use it only at the beginning for $_POST, but is that enough?

Some people suggest that it's best to use it in queries, but I fail to see why.

Zubatman
  • 1,235
  • 3
  • 17
  • 34
Saul Tigh
  • 177
  • 8
  • either / or, as long as it works – Funk Forty Niner Apr 21 '16 at 14:31
  • Obligatory suggestion to switch to at least sqli. – Carcigenicate Apr 21 '16 at 14:31
  • 1
    what's with this *Obligatory* thing? I've yet to find anything on meta about that @Carcigenicate been seeing that often lately. We'll get penalized if we don't? *lol* – Funk Forty Niner Apr 21 '16 at 14:32
  • 1
    *Primarily opinion-based* – Funk Forty Niner Apr 21 '16 at 14:35
  • 1
    @Fred-ii- Not obligatory by SO, no. It was half a joke. Everytime I've ever seen a question where the OP uses the deprecated mysql, there's always a suggestion to switch to either PDO or mysqli. – Carcigenicate Apr 21 '16 at 14:36
  • 1
    @Carcigenicate True. However it seems the OP's stuck at staying with the MySQL_ API, seeing *"Whole website is using mysql so I have to keep it in mysql."*. ;-) – Funk Forty Niner Apr 21 '16 at 14:39
  • 1
    In your trash can next to all other `mysql_*` functions. – Peter Apr 21 '16 at 14:39
  • 2
    `mysql_real_escape_string()` will *escape* quotes in a variable/value. So if those variables (`$email` in this case) are destined for the query, and the query *only*, then passing `$_POST['email']` to `mysql_real_escape_string($_POST['email'])` *prior* to the query is fine. But if you're planning on using those variables *post* query, don't be shocked if your variables now contain escaped quotes, where applicable. – mferly Apr 21 '16 at 14:41
  • 1
    [Little Bobby](http://bobby-tables.com/) says [your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! *Saul can you hear All Along the Watchtower?* – Jay Blanchard Apr 21 '16 at 14:43
  • 1
    Please [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). [These extensions](http://php.net/manual/en/migration70.removed-exts-sapis.php) have been removed in PHP 7. Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [PDO](http://php.net/manual/en/pdo.prepared-statements.php) and [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and consider using PDO, [it's really pretty easy](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Apr 21 '16 at 14:43
  • 1
    For example: if I entered `Marcus's` into an input in a form, and then passed it to `$name = mysql_real_escape_string($_POST['name']);`, then `echo`'d out `$name`, it'd have a value of `Marcus\'s`. So my point is: pass values to `mysql_real_escape_string()` when they're going to be passed to the query, but NOT if you're going to be using those values further in your script (and don't want said values to be escaped, for display purposes). – mferly Apr 21 '16 at 14:44
  • 1
    what I don't get though is; if this is just for email, why not use FILTER_VALIDATE_EMAIL instead? Plus, if you intend on using that with passwords, don't. You're going to get a reality check when that fails you. See this Q&A on the subject http://stackoverflow.com/questions/36628418/cleansing-user-passwords – Funk Forty Niner Apr 21 '16 at 14:49
  • 1
    @JayBlanchard Lol, nothing but the rain. – Saul Tigh Apr 21 '16 at 14:49
  • 2
    *Then grab your gun and bring in the cat.* ;-) – Jay Blanchard Apr 21 '16 at 14:51
  • Yeah well PDO and MySQLi are the luxury I don't have atm. I'll just have to get it secured in a deprecated way :) – Saul Tigh Apr 21 '16 at 14:53
  • 1
    ... @SaulTigh which I knew from the get go ;-) I "read" your question and about *"so I have to keep it in mysql"*. – Funk Forty Niner Apr 21 '16 at 14:53
  • @Marcus can't I just use `str_replace("\\","","$email");` to get the raw email from database stored escaped email? It would basically remove "\" when getting the value out of the database, but not when inserting. – Saul Tigh Apr 23 '16 at 10:26

2 Answers2

3

You should place mysql_real_escape_string() directly into the rubbish bin and migrate to mysqli or PDO and learn to use prepared statements instead.

I've mentioned this before in A Gentle Introduction to Application Security, but the fundamental problem that makes SQL injection possible is the confusion of data and code.

Prepared statements send your query string (SELECT * FROM foo WHERE column = ?) and your parameters (['foo']) in separate packets to the database server. The parameters never get a chance to touch the query string, thus preventing the condition that makes SQL Injection possible in the first place.

Escaping inputs and building the query string does not have the same guarantee. It's possible to do it safely, of course, but if you make one mistake and an unskilled hacker finds it, your entire database is toast. (Keep in mind, SQL Injection is low-hanging fruit.)

TL;DR - Just use Prepared Statements.

Scott Arciszewski
  • 33,610
  • 16
  • 89
  • 206
  • Thanks, but I'm doomed to mysql unfortunately. I'm just not sure if using it at the beginning for example when defining a variable ($email = mysql_real_escape_string($_POST['email']);) is sufficient enough. Or do I have to escape the variable every time it's used? – Saul Tigh Apr 21 '16 at 15:15
  • 1
    What is binding you to `mysql_` functions? – NeoThermic Apr 21 '16 at 15:17
  • the whole site is using mysql_ and I got no time to rewrite 200 pages of code – Saul Tigh Apr 21 '16 at 15:42
  • Guys, chill. Looks like OP already knows about prepared statements. As he says - it is a legacy code. You don't always get to choose how to code, sometimes you need to adjust to what you already have. I never code new projects with `mysql_`, but I still support old projects written with `mysql extension`, and in no way I am rewriting the whole project, not now at least. – Muhammed Apr 22 '16 at 10:14
2

Switch to a better solution other than mysql_ .

That being said, if you have to use the deprecated mysql_, I suggest you use sprintf() for readability and ease of use:

$qemail = sprintf('SELECT email FROM ppl WHERE email="%s"',
                  mysql_real_escape_string($_POST['email'])
                  );

$qr = mysql_query($qemail);

If you have more than one parameter you can have multiple %s and other tags, see sprintf() documentation:

$str_qr = sprintf('SELECT * FROM table, WHERE val="%s", val2="%s", someIntegerField=%d',
      mysql_real_escape_string($val1), 
      mysql_real_escape_string($val2),
      mysql_real_escape_string($someNumberId),
);

$qr = mysql_query($str_qr);
Muhammed
  • 1,592
  • 1
  • 10
  • 18