I'm not sure if this should go in the MySQL board, or this, since it deals with both PHP and MySQL.

But anyway, I am making a sports memorabilia site for a guy. On the item creation page, after I call the function to prevent sql injection, no matter what order I call it, and no matter what other functions I call, it always stores the product description incorrectly. For example, if I include new paragraphs or line breaks in the description text box, and then create the item, on the page the user sees that displays the item, it either shows a /r/n for each new line or line break, or it doesn't show them at all, and treats the entire product description as a single paragraph. On the page that inserts the item into the table, I have tried $productdesc = htmlentities($productdesc), I tried $productdesc = nl2br($productdesc) <- this is the one I used before I put in the code to prevent sql injection, and it worked, but I know I need to prevent sql injection, so I know that something else needs to be done, or done differently.

Before I started doing the sql injection function, it didn't do this, and I used the nl2br function, which seemed to store it in the mysql table just fine, and I would simply use the html_entity_decode function to take away the <br><br> and display a normal new paragraph.

This is the code I'm using to clean any possible sql out of the field:

function cleanQuery($string)
{
  if(get_magic_quotes_gpc())  // prevents duplicate backslashes
  {
    $string = stripslashes($string);
  }
  if (phpversion() >= '4.3.0')
  {
    $string = mysql_real_escape_string($string);
  }
  else
  {
    $string = mysql_escape_string($string);
  }
  return $string;
}

What do you guys think? Is there a way to keep the html line breaks in there while preventing sql injection? I know there has to be some correct way of doing this. I hope I explained this adequately. If not, just ask and I'll try to explain it a different way.

Thanks.

Hi Diode

I think you misunderstand what that function does and how it should be used. The function you are using does not clean out any SQL code. It merely escapes (with a backslash) characters that can be used in SQL injection attacks. If SQL code is being removed, it is not the cleanQuery function that is doing it. That function merely 'escapes' the following characters: \x00, \n, \r, \, ', " and \x1a because of their potential to be used in an attack.

http://www.php.net/mysql_real_escape_string

SQL Injection attacks occur when you allow users to input data that is used to query a database table, such as in a search box or user name and password. It is not absolutely necessary to use it in a secure administration area (although it is still a good idea to use it on all query statements) nor is it intended to be used on fields saved in a table. SQL injection attacks occur when an attacker inputs malicious data that is used in the right side of a MySQL SELECT statement, such as the WHERE clause.

Preventing SQL Injection

With the product maintenance screen you are describing, I assume that you are allowing the site manager to use standard carriage return line breaks between paragraphs, rather than having him or her use HTML code, and the carriage return line breaks do need to be converted to HTML line breaks before you run it through the SQL injection function. You should be able to use the nl2br function to convert the line break characters.

The cleanQuery function should be the last step before you run a query, but it only needs to be used on fields to the right of the WHERE clause. The magic word is QUERY, which means it is used with a SELECT statement.

As far as storing HTML that is under your control, you do not need to convert it to HTML entities. Before storing HTM data in a table, just simply run it through a function that adds escape slashes to the characters (such as single and double quotes) that can break a database. All other HTML characters are stored without any problems.

function escapeString($string)
{
  if (get_magic_quotes_runtime())
  {
    return $string;
  }
  else
  {
    return addslashes($string);
  }
}

You need to check to to see if magic quotes is running or you may add double slashes. If magic quotes is running, it is adding escape slashes automatically. You still need to use a function like this for code portability.

Go to php.net and lookup htmlentities and html_entity_decode methods.

How you display user's input also depends on WHERE you display user's input.

If the user-inputed data from the database is to be later displayed in the web page you want the user's line breaks to become <br>'s, if you are using that input to fill in the value of a form then you will want the \r\n's

Ok so cleanQuotes should only be used during a select statement in the where clause. That makes sense. There are also text boxes where the user logs in. But someone can write up their own custom form and send custom $_POST values to the action page.

So all the values should be cleaned or checked before being added to any table.

TopDogger, are you saying I should use your function, because the one I am using (that I found on the web) strips the slashes, but yours adds them.

I just wondered if there was one simple function that took care of all of this automatically.

It wouldn't really matter if the words drop, insert or whatever are put in the query by a hacker right, as long as the quotes are escaped?

Hi Diode

The purpose for cleanQuery is to protect your data from malicious SQL injection attacks. Those attacks only occur when querying a database. The point is that cleanQuery doesn't strip any code out when you use it for other data fields and using it there will not protect you from an SQL injection attack.

Other data needs to be protected from cross-site scripting, but that is a different issue. Cross site scripting typically includes JavaScript in the user-input data, which can reveal critical data to an attacker. A lot of people get the two attacks confused, but they are very different.


You should add backslashes to escape ALL single and double quotes before they are stored in a table. If you are not actively doing that, then magic quotes is turned on and it is doing it for you.

http://www.php.net/addslashes

Yes, you have to remove the slashes when you display the data. You use stripslashes to do that. Read about it on the official PHP page.

http://www.php.net/manual/en/function.stripslashes.php

TopDogger, are you saying I should use your function, because the one I am using (that I found on the web) strips the slashes, but yours adds them.

Both routines add a backslash when a backslash is needed. Stripslashes is being used in cleanQuery simply to avoid duplicate backslashes. It first strips them out, then adds them back in. If it didn't do that, it would add a second backslash.

Example: \" would become \\" because another backslash would be added before the quote.

The escapeString function doesn't add backslashes unless magic quotes is turned off. If magic quotes is turned on, the backslashes are being added and managed by magic quotes.

The functions are very similar. cleanQuery just escapes a few more characters that could be used in an attack. You may be running into problems with your HTML formatting because cleanQuery is escaping newline (\n) and carriage return (\r) characters that do not need to be escaped for data that is not used as part of a SELECT query. If you make sure that those characters are converted to <br /> code before you use cleanQuery, it probably will not make any difference whether you use one function or the other. cleanQuery has a specific purpose (preventing MySQL attacks with queries) and you are using it for something other than that purpose, which could be the root of your formatting problem.

If you are not already doing so, put all of your functions like these into one PHP file called functions.php and include that file in the top of each of your PHP pages. That way, you never need to duplicate the functions and you just call whichever function is most appropriate for your needs.

BTW, both if (get_magic_quotes_runtime()) and if(get_magic_quotes_gpc()) do the same thing. They just tell you if magic quotes is active.

Does that bring it into perspective? Hope this makes things a little clearer. :)

Comments
He is kind and courteous and he knows his stuff.

Yep, that cleared it up for me. Thanks TopDogger. Btw, I like your username. :)

This question has already been answered. Start a new discussion instead.