SQL Injection

[mattyvx]

In my attempts to protect my database from mySQL injection I have created another problem for myself....

 

Currently all user inputted strings go through this function;

 

function cleanQuery($string)
  {
  if(get_magic_quotes_gpc()) 
    {
    $string = stripslashes($string);
    }
    $string = mysql_real_escape_string($string);

   $string = htmlentities($string);

  return $string;
  } 

 

In the most, its great HOWEVER... there are three fields which I would like the user to be able to enter spaces in. An "About me" field for example, if it is run through the above function the new lines are replaced with a '\r' which i assume is "created" by the mysql_real_escape.

 

Question;

 

1) Should i run the function on every user variable?

2) Is there a safe "fix" or something alternative which i can run on the three fields which may require line breaks.

 

thanks.

 

 

 

 

 

[salathe]

If you put these "problem" values into the database and pull them out again, is there still an issue with "\r"s?

[Anzeo]

Take a look at Mysqli (improved). It will allow you to use prepared statements. In general, this means that you give a standard format for one of your queries and the user input will safely be inserted (no injections are possible!).

 

I do not have much experience with it myself (planning to switch to it soon), but it is one of the safest ways to handle your SQL queries.

[Alex]

When returning the information from the database run it through nl2br before displaying it.

[Anzeo]

When returning the information from the database run it through nl2br before displaying it.

 

or, for that matter, use html_entity_decode

[mattyvx]

I can't seem to get your suggestions to work.

 

To explain further;

 

I'll start with problem 1.

 

The data capture form will take the field values and run them through the function i've posted. If there was an error with the form an error message is shown on the form page and tells the user to edit his/her details to a suitable condition.

 

The form fields values are retrieved by;

 

 

                   
...
<textarea rows="4" name="Areas" id="tswareas" cols="20" ><?php echo html_entity_decode($areas); ?>
...                                                            

 

Now if the user had hit return the text area will now say 'blablabla /r/n' which will more than likely confuse the user which is not what i want.

 

Ideally I would like to store and then later output any return line chars safely, if this isnt possible using a simple method then as a minimum I would like the text area not to display the new line characters.

 

If you put these "problem" values into the database and pull them out again, is there still an issue with "\r"s?

 

No they are not displayed in the database or its resultant output.

 

Thanks.

 

[Anzeo]

Weird, try nl2br?

[cags]

Since the main reason a security function would use htmlentities is to prevent XSS attacks, using html_entity_decode would completely nulify the point in using it. It's the mysql_real_escape_string that is causing it. To output it correctly use...

 

echo str_replace('\r\n', "\r\n", $areas); 

[mattyvx]

AHHH, I think  it's not working because the mysql_real_escape is escaping the nl characters before the nl2br function is run.

 

If i change my function round a little and put the nl2br($string) before the mysqlreales...($string) then it replaces them however it opens up so many more problems.

 

Like if the user needs several attempts to submit the form due to errors the <br /> and new lines keep growing and growing.

 

Cags im trying your suggestion now.

 

[cags]

Whats the problem? The mysql_real_escape_string function has the effect of converting the \r and \n characters into \r\n literals. The code I gave will work perfectly for redisplay on the same form. The code you already had was working fine for database retrieval?!

[mattyvx]

Ok the issue is for the areas field I want the user to be able to have breaks in their text. I.e.

 

I currently serve the following areas;

area a

area b 

 

due to the escaped strings the output im getting is

 

I currently serve the following areas: area a area b

 

Are you with me?

 

 

[cags]

Frankly no. Looking back at your previous posts the problem was you had the characters \r\n where you wanted a newline to be. The code I provided solves that. You are now saying there are no line breaks?!

[mattyvx]

My goals

 

1) If user puts breaks in text and there is an error in form they should not be able to see /r/n characters

2) The outputted string (if breaks are used) must show breaks

 

using

function cleanQuery($string)
  {
  if(get_magic_quotes_gpc()) 
    {
    $string = stripslashes($string);
    }
  
  $string = htmlentities($string);

  $string = mysql_real_escape_string($string);

  $string = str_replace('\r\n', "\r\n", $string); 

  $string = nl2br($string);

  return $string;
  } 

 

solves my goals.

 

However, if there is an error in the form the user can now see <br />

(which is correct for showing text breaks but i'd rather not have the user see it and become confused.)

 

Also now if the form is submitted several times due to errors the number of <br />'s doubles each time.

[cags]

Remove the nl2br and the str_replace from the function. The objective of your function is to sanitise input. Those two lines of code are for formatting output. The contents of a textarea element doesn't use <br> tags, it uses conventional line break characters as such the call to nl2br has no reason to be called for outputting the data back into the textarea box. It should only be called when formatting data for output as part of HTML. The call to str_replace does need to be done for display in a textarea due to the way mysql_real_escape_string works. But you don't want to perform it on data being put into the database only on data being displayed in the textarea, as such you should call it when you output. Pseudo code...