Is this enough?

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
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Is this enough?

Post by psurrena »

For the below code, is mysql_real_escape_string() all I really need or am I missing something?

Code: Select all

<?php
	require '../library/connect.php';
	
	if (isset($_POST['submit'])){
   		$errors = array();
    	if (!$_POST['title'])
			$errors[] = "A job title is required";
		if (!$_POST['description'])	   
			$errors[] = "A job description is required";
   		if (!$_POST['footer'])
			$errors[] = "The footer is required";
		if (count($errors)>0){
			echo "<div id=\"error\">ERROR:<br>\n";
			foreach($errors as $err)
        	echo "$err<br>\n";
        	echo "</div>";
   		} else {
   			$title = mysql_real_escape_string($_POST['title']);
			$location = mysql_real_escape_string($_POST['location'];
			$description = mysql_real_escape_string($_POST['description']);
			$footer = mysql_real_escape_string($_POST['footer']);	

			$query = "INSERT INTO job (title,location,description,footer,date) VALUES ('$title', '$location', '$description', '$footer', current_date)";
			mysql_query($query) or die (mysql_error());
			header ('location: main.php');
		}
	}
	if (isset($_POST['cancel'])){
		header('location:main.php');
	}
?>
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

mysql_real_escape_string() is sufficient in cleaning data being passed into queries.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

The function doesn't clean anything. It merely prepares the information to be added to a query as apart of a string. It can't prevent unexpected data from creating errors in result sets for example.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

feyd wrote:The function doesn't clean anything. It merely prepares the information to be added to a query as apart of a string. It can't prevent unexpected data from creating errors in result sets for example.
Oh! I don't think I've ever been able to ask this question before. Is it better to clean data going into the database, or coming out of the database?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Cleaning needs to be done prior to insertion. Further processing may need to be done on extraction, but only as required to display it in the final destination.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

Okay then. I've always done it going in completely, and almost nothing coming out (aside from little things like nl2br or htmlspecialchars). I liked to be able to just go into my SSH client, run a query, and read the data from there.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

I don't believe it escapes special characters such as % signs which are used in LIKE clauses and things of that nature. But I haven't seen any issues with anything else as of yet.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

You really should be checking isset() or empty() on your variabel comparisons as well.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Usually your first Validate to check if someone is doing something nasty and you want to respond to it. Second filter the data so you only have the type of value in the range/character set/etc. that you want to accept. Finally, you escape when you format the data for an external system such as the browser or database.

You can also Filter first and then Validate second if you want to validate with the assumption that the data is filtered. That can simplify things like forms managers.
(#10850)
Rovas
Forum Contributor
Posts: 272
Joined: Mon Aug 21, 2006 7:09 am
Location: Romania

Post by Rovas »

arborint wrote:Usually your first Validate to check if someone is doing something nasty and you want to respond to it. Second filter the data so you only have the type of value in the range/character set/etc. that you want to accept. Finally, you escape when you format the data for an external system such as the browser or database.
I agree validate using isset, trim and empty. Then filter using functions for special characters
PS Some use regex for validation and filtering. Do a search online to see examples also there are snippets and free code to help you or if you are lazy just use it but I don' t recommend.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

isset(), trim() and empty() are not validators. They simply check for the declaration of a set value in the data, check the presence of data and remove space from the data. Validation needs to go beyond that, like making sure the data is the proper type, within the proper range, etc.

I could be wrong in my view of this however. I may just be getting crossed up in the linguistics of it.
Z3RO21
Forum Contributor
Posts: 130
Joined: Thu Aug 17, 2006 8:59 am

Post by Z3RO21 »

Everah wrote:isset(), trim() and empty() are not validators. They simply check for the declaration of a set value in the data, check the presence of data and remove space from the data. Validation needs to go beyond that, like making sure the data is the proper type, within the proper range, etc.

I could be wrong in my view of this however. I may just be getting crossed up in the linguistics of it.
I agree with you, I always check to see if variables are set (isset()), if they contain information (empty()), and trim out white spaces. I then check data type, length, and special characters with regex.
Post Reply