[RESOLVED] stripslashes & addslashes vs. mysql_real_escape_string

schwim

Hi there guys,

In the code review section of this forum, zabmilenko is offered some suggestions to make my script more secure and be better formed, but I'm having problems finding enough out about some of the items in question to integrate them properly. My first hurdle is mysql_real_escape_string.

zabmilenko states

Instead of using addslashes() for your db input protection, you should use mysql_real_escape_string(). It works with your database server encoding method, which addslashes cannot do.

and

You appear to use addslashes on your db input, then use the inverse stripslashes on the output. In order for this to make sense, you would have to addslashes twice per field. The first time you use addslashes, it would escape the string for the query. The end result of your overuse of stripslashes is parts of the text disappearing if any backslashes are used.

So, I looked it up.

Escapes special characters in the unescaped_string, taking into account the current character set of the connection so that it is safe to place it in a mysql_query(). If binary data is to be inserted, this function must be used.

Now, I use stripslashes and addslashes because if I don't, then my text is messed up. I looked in some other scripts I've used in the past, and found that in their database insertion code.

Here's my current dbinsert block:

	$author = addslashes ($row['author']);
	$author_email = addslashes ($row['author_email']);
    	$authorweb = addslashes ($row['authorweb']);
    	$authorwebname = addslashes ($row['authorwebname']);
    	$title = addslashes ($row['title']);
    	$testtext = addslashes ($row['testtext']);
	$highlight = addslashes ($row['highlight']);

and my retrieval:

		$id = $row['id'];
		$date = $row['date'];
		$author = stripslashes ($row['author']);
		$author_email = stripslashes ($row['author_email']);
		$authorweb = stripslashes ($row['authorweb']);
		$authorwebname = stripslashes ($row['authorwebname']);
		$title = stripslashes ($row['title']);
		$testtext = stripslashes ($row['testtext']);
		$highlight = stripslashes ($row['highlight']);

Should it simply be:

dbinsert:

	$author = mysql_real_escape_string ($row['author']);
	$author_email = mysql_real_escape_string ($row['author_email']);
    	$authorweb = mysql_real_escape_string ($row['authorweb']);
    	$authorwebname = mysql_real_escape_string ($row['authorwebname']);
    	$title = mysql_real_escape_string ($row['title']);
    	$testtext = mysql_real_escape_string ($row['testtext']);
	$highlight = mysql_real_escape_string ($row['highlight']);

retrieval:

	$author = $row['author'];
	$author_email = $row['author_email'];
    	$authorweb = $row['authorweb'];
    	$authorwebname = $row['authorwebname'];
    	$title = $row['title'];
    	$testtext = $row['testtext'];
	$highlight = $row['highlight'];

Or does the fact that magic quotes can mess this whole thing up require me to use something like what was on the php.net site?

<?php
// Quote variable to make safe
function quote_smart($value)
{
   // Stripslashes
   if (get_magic_quotes_gpc()) {
       $value = stripslashes($value);
   }
   // Quote if not a number or a numeric string
   if (!is_numeric($value)) {
       $value = "'" . mysql_real_escape_string($value) . "'";
   }
   return $value;
}

// Connect
$link = mysql_connect('mysql_host', 'mysql_user', 'mysql_password')
   OR die(mysql_error());

// Make a safe query
$query = sprintf("SELECT * FROM users WHERE user=%s AND password=%s",
           quote_smart($_POST['username']),
           quote_smart($_POST['password']));

mysql_query($query);
?>

What concerns me is that the script is available for public download, so although I may have magic_quotes set to where this will work, the next guy in line might not. The function from the php.net site confuses me more than it helps, because I need the slashes added & stripped, regardless of whether I'm dealing with a number or not. Else, I\'ll have text that doesn\'t look really hot.

thanks,
json

sneakyimp

PART 1 - magic quotes
basic idea is that if 'magic quotes' is on then PHP will add backslashes to ' and " when it is getting it from GET, COOOKIE, or POST (that's what the 'gpc' in 'get_magic_quotes_gpc()' stands for.

What this means is that when users POST or GET a form to you, your server might add backslashes to those particular chars.

PART 2 - Why??
Because PHP was designed with some shortcuts for dumb people (not you) to get started quickly and somebody thought it would be convenient to auto-add backslashes so the user didn't have to before creating a query. If you are trying to insert a string with quotes into a database, you can probably see why this wouldn't be valid sql:

$sql = "INSERT INTO table SET name='O'Malley'";

that apostrophe in O'Malley breaks the SQL. so you would put a backslash before it to fix it.

PART 3 - detecting and getting rid of those stupid, useless and totally non-magical 'magic_quotes'

this line is good...

   // Stripslashes
   if (get_magic_quotes_gpc()) {
       $value = stripslashes($value);
   }

it will remove slashes that are added by magic quotes, but NOT ones that might have been entered by the user....like when i sit here and type \'
I want it in my post here as an example to describe something to you.

It will also not bother removing magic_quotes if your php install DOESN'T HAVE THEM TURNED ON.

stripslashes() is effective for removing magic quotes. so that kind of line is good.

PART 4 - Escaping strings properly before putting into SQL
Why use mysql_real_escape_string() instead of addslashes?

BECAUSE, depending on what language settings your MySQL installation has going on, there are OTHER LANGUAGE SYMBOLS THAT MYSQL TREATS AS DOUBLE OR SINGLE QUOTES. There's that angle quote that phpMyAdmin uses all the time: `(shares a key with the tilde char up by your escape key).

There are others like curly quotes left and right. curly double quotes left and right, etc. Depending on what language setting your MySQL has, there could be a dozen for all I know. addslashes() doesn't affect those at all as far as i know. mysql_real_escape_string should - i think so anyway.

Remove magic quotes (if the setting is turned on) and use mysql_real_escape_string instead of addslashes.

PART 5 - Don't stripslashes unnecessarily
This might be kind of confusing, but when you escape all those quote marks when you are creating your SQL, don't worry! The contents of your database SHOULD NOT HAVE ANY SLASHES THAT NEED TO BE STRIPPED IF YOU HAVE DONE YOUR WORK PROPERLY. For this reason, you should NOT stripslashes() when you retrieve information from your database. If you are getting all kinds of when you pull data from your database, you didn't insert it properly in the first place.

Again, The reason that you add the slashes in the first place is to make valid SQL. When you run that valid SQL, the backslashed quote marks don't go into the database with the backslashes. You just backslash them so that the SQL isn't broken. Is that clear? Look at the O'Malley example above. What I typed there is not valid SQL. If I 'escaped' that one quote in O'Malley then it would be valid SQL

$name = "O'Malley";
// or i think i could to this...
$name = 'O\'Malley'; // notice the backslash...the var still just contains O'Malley...we just have to escape the quote to make valid php...the idea for sql is the same

// this line will add a backslash to O'Malley but that's just so the SQL isn't broken when we try to run the query...it doesn't actually go into the database
$sql = "INSERT INTO table SET name='" . mysql_real_escape_string($name) . "'  ";
mysql_query($sql)

If I were to retrieve that record and echo the name value from it, it would be O'Malley (without backslashes).

gosh that is a lot of typing. hope this is clear.

schwim

sneakyimp, I don't think I could put into words how much that helped. After reading your explanation, I realized that I hadn't even taken the time to find out what magic quotes did before I started learning how to combat it.

Not to consolidate your post, but just so you don't shoot me for doing another long post:

addslashes during table insertion creates proper sql queries. If an entry has a quote, the instruction ends prematurely('O'Malley' becomes 'O').

I don't need stripslashes during data retrieval, because addslashes only helped create proper queries. The data saved is as it needs to be. If it's not, then I've messed something up on insertion.

I may be totally overthinking this, but is this what I need to do?

Function:

function strip_magicquotes(){

   if (get_magic_quotes_gpc()) {
       $value = stripslashes($value);
   } 
}

Inserting my data:

$data = mysql_real_escape_string ($row['data']);

And retrieval:

$data = strip_magicquotes($row['data']);

Don't think I'm not paying attention if I got this wrong, I'm just trying to combine all aspects of the data circle 🙂

Thanks again so much for your explanation. The only down side to this is that I did the whole of my scripts db work incorrectly 🙂

thanks,
json

zabmilenko

Skip the retrieval stripping. Your insert step and cleanup function will be enough.

schwim

Thanks very much zabmilenko 🙂 I was afraid if I started asking you all of the questions in my review thread, you would feel trapped...

I'm going to try to fix the items you mentioned before posting there again.

thanks,
json

sneakyimp

that all sounds pretty good EXCEPT for the retrieval. You probably don't need to strip any backslashes from information that comes out of your database. You can probably just retrieve it and use it.

As it turns out, there are TWO DIFFERENT KINDS OF MAGIC QUOTES. Rats.

1) magic_quotes_gpc
this is an INI setting for your php that you may or may not be able to change. It affects ONLY information that you receive in your script from GET, POST, or COOKIE. You can check the current setting for it using get_magic_quotes_gpc() just like you did in your strip_magicquotes() function.

BAD NEWS: You can't turn this setting on or off unless you can either access PHP.INI or if your PHP setup is configured to let you specify its value in an .htaccess file.

GOOD NOWS: You can reverse it using a function just like your strip_magicquotes() function.

REMEMBER this only applies to GET/POST/COOKIE
IMPORTANT - that setting will have no impact at all on information retrieved from a database.

2) magic_quotes_runtime
If THIS setting is on, backslashes will be added to data retrieved from a database or read from a file. You can check its value with get_magic_quotes_runtime(). From the php docs:

If magic_quotes_runtime is enabled, most functions that return data from any sort of external source including databases and text files will have quotes escaped with a backslash. If magic_quotes_sybase is also on, a single-quote is escaped with a single-quote instead of a backslash.

GOOD NEWS: You can also TURN IT OFF using set_magic_quotes_runtime(0) before you do any file reads or database queries then you don't have to worry about it.
MORE GOOD NEWS: It's usually off as far as I can tell, but you never know.

If you're really anal, you might want to create a different function for stripping the 2nd kind of magic quotes from information that comes from a file:

function strip_magicquotes_runtime(){

   if (get_magic_quotes_runtime()) {
       $value = stripslashes($value);
   }
}

Better yet, if you have some file that gets included by all of your PHP files, you can add this line there:

set_magic_quotes_runtime(0);

schwim

Hey there sneakyimp,

thanks very much for your reply.

So should I turn of magic_quotes_runtime off via the script, and use the magicquotes_strip function for any data being retrieved from GET, POST & COOKIE? Or are you saying that I should drop the function altogether? It seems that turning runtime off will take care of it in regards to retrieving data from the db, but should I keep the function for my forms?

thanks,
json

schwim

Well, I guess I didn't latch onto this concept as well as I had initially though 🙁

Here's what I did:

In my submittal form:

$text = mysql_real_escape_string ($_POST['text']);

In my retrieval page:

$text = $row['text'];

I entered:

I'm testing the new escape method.

\\ <~ three slashes here
/// <~ three slashes here
''' <~~~ three apo's~~


We'll see how it goes.
[/quote]

I got this when I retrieved it:
[quote]
I\'m testing the new escape method.

\\\\\\ <~~~ three slashes here
/// <~~~ three slashes here
\'\'\' <~~~ three apo\'s
``` <~~~ three accents.

We\'ll see how it goes.
[/quote]

I tried the strip_magicquotes function, but I must have written it incorrectly, because when I use it, I'm told that $value is undefined.

Are the rest of my changes correct, aside from my b0rked gpc attempt?

thanks,
json

sneakyimp

schwim wrote:
Hey there sneakyimp,

thanks very much for your reply.

So should I turn of magic_quotes_runtime off via the script, and use the magicquotes_strip function for any data being retrieved from GET, POST & COOKIE? Or are you saying that I should drop the function altogether? It seems that turning runtime off will take care of it in regards to retrieving data from the db, but should I keep the function for my forms?

thanks,
json

turn off magic quotes runtime using set_magic_quotes_runtime(0) BEFORE you retrieve any info at all from a database or file. just to be on the safe side. Like I said, it's usually off but you can't be too careful.
Use your function to strip the magic quotes from GET/POST/COOKIE before you do anything with it at all, whether it's echo it in a form or put it in a query statement.
since all your data from GET/POST/COOKIE is now naked quotes without any backslashes, make sure you use mysql_real_escape_string() before trying to put it into a query.

that said, your approach in the last post you did doesn't appear to strip any magic quotes from your POST information before you run mysql_real_escape_string() on it. I also just noticed your strip_magicquotes function doesn't have any parameters defined. The parenetheses are empty!

try this:

// this fn will only strip magic quotes if they have been added
// otherwise, doesn't touch the value
function strip_magic_quotes_gpc($arg) {
  if (get_magic_quotes_runtime()) {
    return stripslashes($arg);
  } else {
    return stripslashes($arg);
  }
} // strip_magic_quotes_gpc()


// strip the magic quotes from GET/POST/COOKIE (if there are any!);
$text = strip_magic_quotes_gpc($_POST['text']);

$query_ready_text = mysql_real_escape_string ($text);

$sql = "INSERT INTO table SET text='" . $query_ready_text . "' ";
mysql_query($sql)
  or die('query failed:' . mysql_error());


// now fetch it
$sql = "SELECT text FROM table";
$result = mysql_query($sql)
  or die ('select query failed:' . mysql_error());

$row = mysql_fetch_assoc($result)
  or die('nothing fetched');

echo $row['text'];

schwim

Hi there again,

Sorry if I'm being overly dense about this, but:

function strip_magic_quotes_gpc($arg) {
  if (get_magic_quotes_runtime()) {
    return stripslashes($arg);
  } else {
    return stripslashes($arg);
  }
}

Aren't we telling it to stripslashes in both instances, regardless of whether magic_quotes_gpc is on or not, or am I misunderstanding the function?

thanks,
json

Weedpacket

Yes, that's a typo. You'd return it unchanged if there are no magic slashes to strip.

One more piece of info: magic quotes are dead as of PHP 6.

sneakyimp

oops! sorry. weedpacket's right:

function strip_magic_quotes_gpc($arg) {
  if (get_magic_quotes_runtime()) {
    return stripslashes($arg);
  } else {
    return $arg;
  }
}

PHP 6???? dANG! I'm way behind.

zabmilenko

One of my favorite pages nowadays: http://www.php.net/~derick/meeting-notes.html

schwim

Ok, I've fixed the function, but have a question:

You state:

// strip the magic quotes from GET/POST/COOKIE (if there are any!);
$text = strip_magic_quotes_gpc($_POST['text']);

$query_ready_text = mysql_real_escape_string ($text);

$sql = "INSERT INTO table SET text='" . $query_ready_text . "' ";
mysql_query($sql)
  or die('query failed:' . mysql_error());

Since it all has to be done before inserting, can I do this?

$text = mysql_real_escape_string (strip_magic_quotes_gpc ($_POST['text']));

or is there something wrong with that?

thanks,
json

schwim

Ok, I don't want to jinx myself, but I made the changes I noted above, and entered:

I'm Bold!

/// < three slashes
\\ < three backslashes
''' < three apo's
""" <three quotes

We'll see how this works.

thanks,
json

And I received

I'm Bold!

/// < three slashes
\\ < three backslashes
''' < three apo's
""" <three quotes

We'll see how this works.

thanks,
json

My function:

function strip_mq_gpc($arg) {
  if (get_magic_quotes_gpc()) {
    return stripslashes($arg);
  } else {
    return $arg;
  }
}

My insert:

	/*Action to submit changes from the testimonial edit form*/
	case "edit_submit" :

$id=($_POST['id']);

$author = strip_tags (mysql_real_escape_string (strip_mq_gpc ($_POST['author'])));
$author_email = strip_tags (mysql_real_escape_string (strip_mq_gpc ($_POST['author_email'])));
$authorweb = strip_tags (mysql_real_escape_string (strip_mq_gpc ($_POST['authorweb'])));
$authorwebname = strip_tags (mysql_real_escape_string (strip_mq_gpc ($_POST['authorwebname'])));
$title = strip_tags (mysql_real_escape_string (strip_mq_gpc ($_POST['title'])));
$testtext = mysql_real_escape_string (strip_mq_gpc ($_POST['testtext']));
$highlight = mysql_real_escape_string (strip_mq_gpc ($_POST['highlight']));

mysql_query("UPDATE ".$prefix."testimonials SET author = '$author', author_email = '$author_email', authorweb = '$authorweb', authorwebname = '$authorwebname', title = '$title', testtext = '$testtext', highlight = '$highlight' WHERE id = $id") or die("Cannot query the database.<br><br>" . mysql_error());


echo("<div align=center>
<table width=400 border=1 cellpadding=1 cellspacing=1>
	<tr>
		<td bgcolor='#ffffff'>
		<img src='../images/icons/important-lg.png'>The testimonial has been updated.
		</td>
	</tr>
</table>
</div>");

break;

My retrieval:

	/*Grabbing the necessary information to populate the approved testimonials page*/

$query = "SELECT id,active,author,author_email,authorweb,authorwebname,title,testtext,highlight," .
"DATE_FORMAT(postdate, '%M %D, %Y') as date " .
"FROM ".$prefix."testimonials WHERE active=1 ORDER BY postdate DESC";
$result = mysql_query($query) or die('MySQL error: ' . mysql_error() . '<hr/>' . $query);
while ($row = mysql_fetch_assoc ($result)) {

	/*Display the data*/

	$id = $row['id'];
	$date = $row['date'];
	$author = $row['author'];
	$author_email = $row['author_email'];
	$authorweb = $row['authorweb'];
	$authorwebname = $row['authorwebname'];
	$title = $row['title'];
	$testtext = $row['testtext'];
	if($use_highlight==1){
		$highlight = $row['highlight'];
	}

Before I mark the thread resolved, I was wondering if anyone sees anything else I should be taking care of(in regards to formatting OR security).

Thanks so much to all of you for your help.

thanks,
json

sneakyimp

Just make sure it preserves stuff. Try entering some text with backslashes in front of quote marks like \' or \"

schwim

Well, I think I can put it to rest.

entered

Testimonial Content:

Testing the new slash function.

I\'m an apostrophe with a slash in front of it!

I'm an apostrophe without!

\" I'm a quote with a slash in front of it!

" I'm a quote without!

\|||///

and got

Testimonial Content:

Testing the new slash function.

I\'m an apostrophe with a slash in front of it!

I'm an apostrophe without!

\" I'm a quote with a slash in front of it!

" I'm a quote without!

\|||///

I think that's all I would need to test for.

Thanks again to all of you for your help. It was invaluable.

thanks,
json

sneakyimp

glad to help. i once had the same questions.

MarkR

In the grand scheme of magic quotes, addslashes, stripslashes and mysql_real_escape_string, my opinion is:

If magic_quotes_gpc or magic_qutoes_runtime is enabled, have the application crash out with an error message. Then you never have to worry about the being on (fewer "if" statements == better test coverage, fewer bugs)
Don't manually escape any strings in SQL at all if possible, instead use parameterised queries (via PDO, mysqli, PEAR:😃B etc)
If at some point it becomes absolutely necessary to place some data inside an SQL statement (yes, sadly this does happen occasionally), then use the right escaping method for your database access layer, which MAY actually be mysql_real_escape_string but probably not ($db->quote is more likely).
Because you should never be using addslashes() or stripslashes(), you may as well use disable_functions in php.ini to disable them so that you get a fatal error if you ever use them by accident.

Then you can rest assured that you won't get any of these problems.

Preventing data corruption is FAR more important than making installation easy for idiots.

Mark