Data to mysql that doesnt use mysql_real_escape_string

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
danf_1979
Forum Commoner
Posts: 72
Joined: Sun Feb 20, 2005 9:46 pm

Data to mysql that doesnt use mysql_real_escape_string

Post by danf_1979 »

This code is from e107 CMS. It is used everytime a data must go to mysql. I got some questions about it:
1) Why they use htmlspecialchars and not mysql_real_escape_string?
2) Is this code worthy?
$search = array('$', '"', "'", '\\', '<?');
$replace = array('$','"',''', '\', '&lt?');
$ret = str_replace($search, $replace, $data);

3) And this one?
$data = htmlspecialchars($data, ENT_QUOTES, CHARSET);
$data = str_replace('\\', '\', $data);
$ret = preg_replace("/&#(\d*?);/", "&#\\1;", $data);

I ask because I use mysql_real_escape_string, and I have been adviced to do so, by several people I think they know what they're talking about. So please.. could you comment on this? Does this code have any advantage over mysql_real_escape_string?

Code: Select all

function toDB($data, $nostrip = false, $no_encode = false, $mod = false)
	{
		global $pref;
		if (is_array($data)) {
			// recursively run toDB (for arrays)
			foreach ($data as $key => $var) {
				$ret[$key] = $this -> toDB($var, $nostrip, $no_encode);
			}
		} else {
			if (MAGIC_QUOTES_GPC == TRUE && $nostrip == false) {
				$data = stripslashes($data);
			}
			if(isset($pref['post_html']) && check_class($pref['post_html']))
			{
				$no_encode = TRUE;
			}
			if (getperms("0") || $no_encode === TRUE)
			{
				$search = array('$', '"', "'", '\\', '<?');
				$replace = array('$','"',''', '\', '&lt?');
				$ret = str_replace($search, $replace, $data);
			} else {
				$data = htmlspecialchars($data, ENT_QUOTES, CHARSET);
				$data = str_replace('\\', '\', $data);
				$ret = preg_replace("/&#(\d*?);/", "&#\\1;", $data);
			}
		}
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

mysql_real_escape() should be used for a couple reasons, although it basically boils down to the fact this function escapes more than just quotes. It also escapes

Code: Select all

NULL, \x00, \n, \r, \, ', " og \x1a
htmlspecialchars() will not, therefor the possibility of SQL injection is technically still possible.

It is my preference to never input any formatted information, considering how I handle the data can be easily changed without having to ever reformat the information into the database. It is also best to use htmlentities()/htmlspecialchars() outputting the dataset to prevent XSS attacks.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

don't use regular expressions where standard functions will do. standard functions will always work and they better describe the action being taken.
baggypants303
Forum Newbie
Posts: 9
Joined: Sat Apr 01, 2006 11:04 am

Post by baggypants303 »

what is this: \x00, og, and \x1a ????
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

\x00 = null byte

I'm not sure what the other two are. Why don't you get an ASCII character chart and look them up?
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

They are using that because versions of PHP before 4.3.0 do not support mysql_real_escape_string.

Instead of actually checking the version of PHP the program is running on and uses the appropriate code they just using the old coding method. This way their program will work on older versions of PHP.

They could use something like this that would work on older and newer versions of PHP.

Code: Select all

if (strnatcmp(PHP_VERSION, '4.3.0') >= 0) {
	return "'" . mysql_real_escape_string($string) . "'";
}
else
{
	$string = str_replace("'", "\\'" , str_replace('\\', '\\\\', str_replace("\0", "\\\0", $string)));
	return  "'" . $string . "'"; 
}
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

btw AKA Panama Jack you might be interested to learn about version_compare()
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

Yep, I knew about that one but it is also specific to PHP 4.1.0 and higher. The one I posted is generic and should work on any version of PHP.
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

It's careless - report it as a security bug and tell them their escaping is a half measure that's open to exploitation (might be - depends on the workflow to that point...).

ADOdb suffered from the same fatal mistake up to a short time ago. It used a custom escape function for strings in PostgreSQL, completely ignoring pg_escape_string(). It was ignored for months (no kidding) after I reported it. I doubt I was the only one to. Eventually public highlighting forced some action (it was reported openly on several major security listings).

If applications ceased considering these half measures "secure", it would solve a lot of problems...
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

It's careless -
i'm sorry i'm lost, what's careless exactly?
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

It's careless relying on a custom escaping function,when more modern PHP versions have native functions like mysql_real_escape_string() which escape more accurately and completely.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

oh right. well then i'm in total agreement.
Post Reply