0

I have a function in my function library called sanitse. It takes one argument of a string and returns the sanitised string back. 

function sanitise($data){
    return htmlentities(mysql_real_escape_string($data));
}

I am developing a forum and I need to sanitise data that is submitted as text. (it is stored in a database). There are problems arising though. When the data is stored new lines are represented as \r\n and that is appearing on the thread after it is retrieved. There should be a new line instead. nl2br doesn't sort it out and I read on another post that this should be tackled at the source of the problem (the function) rather than 'hacking' a solution to correct it.

If I don't sanitise the data, the comment is displayed correctly. But that is of course not secure.

Any suggestions would be gratefully received.

jollinski
  • 23
  • 1
  • 5

1 Answers1

2

What you are doing isn't secure anyway. You really should understand what you are doing.

mysql_real_escape_string(): This function takes a string and escapes it so it can be used within an SQL statement. All this does is make it clear to the query parser what is data, and what is the query. It is important you understand this.

htmlentities()/htmlspecialchars(): These functions escape strings so that they can be used within HTML without the HTML parser thinking that characters such as < or " are HTML. These entities become &lt; and &quot;.

You must use these functions in the correct context. By not using mysql_real_escape_string() as the last thing that touches a string before it is used in a query context, you negate most of its value. You should only use this function when inserting data into a database, and only when the string is fully formed. The best thing to do is not use data within your queries at all. Use PDO with prepared/parameterized queries instead. This effectively removes the data from the query, eliminating any chance of SQL injection.

You should not be using htmlentities() until you are ready to insert arbitrary strings into HTML. That is, do not use this function before you put stuff in your database. Otherwise, you make your data really hard to work with in any other context. It is a real hassle dealing with the mess people make that do this. Don't do it.

Now, if you are seeing a literal \r or \n in your text, then that data is coming from something else. Not from the two functions you are using. Neither of those functions will insert such a string.

See also: https://stackoverflow.com/a/7810880/362536

Community
  • 1
  • 1
Brad
  • 159,648
  • 54
  • 349
  • 530
  • Thanks for your response. So in that case swapping the mysql_real_escape_string and htmlentities would make the code safe, provided of course that the function is called as the last thing before the mysql injection? – jollinski Mar 15 '13 at 03:50
  • @jollinski, Yes, but don't do it! Switch to PDO for your database connections, use prepared queries, and escape with `htmlspecialchars()` upon output of any variable data in HTML. Then, you won't have to worry about these problems so much. Otherwise, you are just making a mess of your data. – Brad Mar 15 '13 at 03:54