13

I would like to start my question by saying, I realize PDO/mysqli is the new standard and has been widely covered on SO. However in this particular case I dont have time to convert all queries to PDO before launching the clients site.

The following has been used throughout most of the queries on the site (not by me may I add)

  $userEmail = filter_var($_POST['fEmail'], FILTER_SANITIZE_EMAIL);
   $userEmail = mysql_real_escape_string($userEmail);
   $sql ="SELECT email FROM members WHERE email = '$userEmail'";
   :
   :

I would like to know:

Is it good / okay practise to use filter_var and mysql_real_escape_string together as in the example above? My main concern is, can these two functions be used together or cause some sort of conflict / bug when executing / uploading to DB?

Also is there any sort of benefit in using both?

Thanks in advance

SuperDJ
  • 7,488
  • 11
  • 40
  • 74
Timothy Coetzee
  • 5,626
  • 9
  • 34
  • 97
  • 1
    @Your Common Sense with the risk of sounding like a school boy: in my question I mentioned `it was not me`...Im just scanning over the code and correcting/changing a few minor things before launch – Timothy Coetzee Sep 03 '15 at 14:10
  • Plus, the question received a downvote; which in my view was uncalled for. Had a certain someone "read" and "understood" what seems to be "clear/plain English" and in regards to the use of the MySQL API, then they'd quickly double back with their tail between their legs, or other means of running away. Some walk differently than other animals. – Funk Forty Niner Sep 03 '15 at 14:32

4 Answers4

16

Sanitising a string serves to make it conform to certain expectations. FILTER_SANITIZE_EMAIL removes any characters from a string which would be invalid in an email. The result is (supposedly) guaranteed to conform to email address syntax. How useful randomly removing characters from a string is I'll leave up to you. (Hint: I don't think it's very useful at all; you should rather reject invalid addresses than to transform them into random results. I give you an invalid email address, you hammer it into some shape that resembles an email address, now how do you know you'll be able to send me an email...?!)

mysql_real_escape_string is there to ensure that an arbitrary string does not violate SQL's string literal syntax by escaping all escape-worthy characters. Assuming you're using it correctly (lots of pitfalls mysql has, which is why it's deprecated...), there's nothing you can do to its input that would make it fail. You give it any arbitrary string, it returns you the escaped version, period.

As such, in general, yes, what you're doing is fine. If mysql_real_escape_string is the last thing you do to your string before interpolating it into an SQL string literal, then it's fine.

deceze
  • 510,633
  • 85
  • 743
  • 889
  • 4
    @Fred-ii- The radical anti-mysql faction. Yes, I'm looking at you, Col. Shrapnel. ;-) – deceze Sep 03 '15 at 14:18
  • 1
    My *spidey sense was tingling* as to a certain reptile roaming about. Yeah, I'm with you on this, *my sentiments exactly*. – Funk Forty Niner Sep 03 '15 at 14:18
  • 1
    For the record: yes, absolutely, `mysql_real_escape_string` is a terrible approach to the whole problem space... But the OP knows and this caveat is factored into the question and answer... – deceze Sep 03 '15 at 14:20
  • for what it is worth you got my vote thank you for the info – Timothy Coetzee Sep 03 '15 at 14:20
  • 1
    @deceze Indeed. Had people actually "read" what the OP wrote in regards to their use of the MySQL API, then there wouldn't be all this confusion. – Funk Forty Niner Sep 03 '15 at 14:21
3

In this case, if the filter fails the query will be:

SELECT email FROM members WHERE email = '0'

So, not much to worry about the query; it just won't return any results.

Unless of course, you have a similar insert / update query.

In which case, you could have plenty of rows in the database where the email is '0'

Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
andrew
  • 9,313
  • 7
  • 30
  • 61
2

Using both is fine, but what you should really be doing is not to use mysql_ at all. You should instead use PDO or mysqli_ with prepared statements to avoid SQL injection.

See How can I prevent SQL injection in PHP?

Or for a guide of how to switch to MySQLi the see Can I blindly replace all mysql_ functions with mysqli_?

Community
  • 1
  • 1
worldofjr
  • 3,868
  • 8
  • 37
  • 49
-3

Your main concern should be this outdated approach, which is leaving your client's site vulnerable to SQL injection attack. While regarding this trifle matter of using yet another irrelevant function, there is nothing to worry about.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • 3
    Your answer might actually be considered useful if you could show why this would be a vulnerability issue (since that *is* what the OP is concerned about), either a quotation from a reputable source or an example as to how to bypass whatever it is that mysql_real_escape_string pretends to do. – cimmanon Sep 03 '15 at 17:07