5

I'm using mysqli prepared statements. Should I still sanitise the user input with some function like:

function sanitise($string){
  $string = strip_tags($string); // Remove HTML
  $string = htmlspecialchars($string); // Convert characters
  $string = trim(rtrim(ltrim($string))); // Remove spaces
  $string = mysql_real_escape_string($string); // Prevent SQL Injection
  return $string;
}

Thanks.

Jay
  • 10,831
  • 9
  • 26
  • 33
  • 1
    If you're injecting the input values directly into your SQL statement, then "yes". If you're using bind variables, this won't strip html or convert characters or trim... but it will prevent SQL injection – Mark Baker Mar 09 '11 at 00:25
  • 14
    The `trim(rtrim(ltrim($string)))` is most amusing. – mario Mar 09 '11 at 00:27
  • @mario: I found this function on a website, just copied here. – Jay Mar 09 '11 at 00:29
  • 13
    @adam Never visit that website again. – alex Mar 09 '11 at 00:29
  • http://techpad.co.uk/content.php?sid=86 – Jay Mar 09 '11 at 00:34
  • @adam: That article seems like it tried to list alternatives. You only need the `->bind_param()` as listed in the mysqli section. The `strip_tags` and `htmlspecialchars` are necessary for later output, but not database security. (It doesn't hurt, but storing content pre-encoded is not considered best style.) – mario Mar 09 '11 at 00:38
  • If I didn't have to signup to leave comments on that article I would :D – Jacob Mar 09 '11 at 00:40

5 Answers5

8

No! No and no. If you are already using prepared statements, MySQL needs to see the value, not some escaped version of it. If you add mysql_real_escape_string to a string and make that the value for a prepared statement, you have just junked it, for example, quotes get doubled up!

Now, as for sanitising data-wise, that's entirely up to the business rules as to what is or is not valid input. In your example, strip_tags is more about html->raw (format) conversion than sanitation. So is rtrim(ltrim - this is a business transformation.

RichardTheKiwi
  • 105,798
  • 26
  • 196
  • 262
  • "If you add mysql_real_escape_string to a string and make that the value for a prepared statement, you have just junked it" <-- good one. – tacone Mar 09 '11 at 00:37
2

Yes. When using prepared statements you are safe from mysql injections, but still there could be special characters, strip tags or spaces, so those you will still need to take care of those.

See PHP: Is mysql_real_escape_string sufficient for cleaning user input?

UPDATE:

You are safe from mysql injections so you should not use real_mysql_scape_string or scape any quotes.

Community
  • 1
  • 1
amosrivera
  • 26,114
  • 9
  • 67
  • 76
0

Prepared statements are there to keep your query form being subverted by malicious input. But there's plenty of malicious content that is perfectly acceptable in an SQL query, but will attack a browser when redisplayed later.

Doing mysql_real_escape_string on data going into a prepared statement is generally redundant (there are exceptions, but they're special-ish cases).

Marc B
  • 356,200
  • 43
  • 426
  • 500
0

Here is an Object orientated solution to your question:

public function sanitize($var){
    if(is_array($var))
        foreach($var as $k => $v) $var[$k] = $this->db->real_escape_string($v);
    else
        $var = $this->db->real_escape_string($var);
    return $var;
}
Dharman
  • 30,962
  • 25
  • 85
  • 135
Marcello Perri
  • 572
  • 7
  • 22
-1

You should always sanitize your user inputs before submitting them to the database. I would just stick with mysql_real_escape_string as the others are not that much necessary unless you are putting them back on the URL.

Rasika
  • 1,980
  • 13
  • 19
  • As @RichardTheKiwi said: "If you are already using prepared statements, MySQL needs to see the value, not some escaped version of it. If you add mysql_real_escape_string to a string and make that the value for a prepared statement, you have just junked it, for example, quotes get doubled up!" – NBTX Apr 15 '17 at 16:06