Script Security Issues

Hi, i’m pretty new to all this programming stuff and have cobbled the following code together using tutorials i’ve found on the internet…

It’s basically a pull down list that uses a database to control the options in a list and then redirects the user depending on what they’ve selected. Now, i’ve made all the variables that i’m using in the queries safe using “mysql_escape_string” but i’d like some help on how to make the script safe before i put it online…

I don’t expect anyone to do all the work for me but some tips/guidance would be helpful. I know some of the code i’ve used might not be the best practise but before i confuse myself too much i’d like some tips on securing what i do have.


//Redirect to search results if form submitted
if (isset($_POST['submitted'])) {
	 // Connect to the database and run the query
  $dbid = mysql_connect ('localhost', 'xxxxx.xxxxx', 'xxxxx.xxxxx');
	          or die ("Cannot find database");
	 $safe_cat = mysql_escape_string(trim($cat));
	  $cat_query = "SELECT * FROM category WHERE cat_id=$safe_cat";
	  $result = mysql_query($cat_query,$dbid) 
	    or die("SELECT error:".mysql_error());
	 $category_name = mysql_num_rows($result);
	 while($row = mysql_fetch_array($result))
	 $make = $row['category'].'+';
	 $model = $subcat;
	 $safe_make = mysql_escape_string(trim($make));
	 $safe_model = mysql_escape_string(trim($model));
	 //echo $make;
	 //echo '<br>';
	 //echo $model;
	 header('Location:' . $safe_make . $safe_model);

//Database Details
//Database Username & Password
//Database Name

//Database Query
function connecttodb($servername,$dbname,$dbuser,$dbpassword)
	{	global $link;
		$link=mysql_connect ("$servername","$dbuser","$dbpassword");
		if(!$link){die("Could not connect to MySQL");}
			mysql_select_db("$dbname",$link) or die ("could not open db".mysql_error());

<!doctype html public "-//w3c//dtd html 3.2//en">
<SCRIPT language=JavaScript> 
	function reload(form)
		{ var[].value;
			self.location='dd.php?cat=' + val ;
//If register_global is off in your server then after reloading of the page to get the value of cat from query string we have to take special care.

// Use below line if register_global is off 

//First Pulldown Query Begin
$quer2=mysql_query("SELECT DISTINCT category,cat_id FROM category order by category"); 
//First Pulldown Query End

//Second Pulldown Query Begin
//This is only run if the first category has been selected
if(isset($cat) and strlen($cat) > 0){
$quer=mysql_query("SELECT DISTINCT subcategory FROM subcategory where cat_id=$cat order by subcategory"); 
}else{$quer=mysql_query("SELECT DISTINCT subcategory FROM subcategory order by subcategory"); } 
//Second Pulldown Query End

//Center All Elements <DIV> Begin
echo "<div style="text-align: center;"> ";

echo "<form enctype="multipart/form-data" method="post" name="submitted" action="dd.php">";

//Display First Pull Down Form Begin
echo "<select style='width: 150px;' name='cat' onchange="reload(this.form)"><option value=''>Make</option>";
	while($noticia2 = mysql_fetch_array($quer2)) { 
		if($noticia2['cat_id'][email protected]$cat){echo "<option selected value='$noticia2[cat_id]'>$noticia2[category]</option>"."<BR>";}

		else { echo  "<option value='$noticia2[cat_id]'>$noticia2[category]</option>"; }
					echo "</select>";
					echo "<br>";
					echo "<img src="/xxxxx.xxxxx/images/spacer_4x4.gif">";
					echo "<br>";
//Display First Pull Down Form End

//Display Second Pull Down Form Begin
echo "<select style='width: 150px;' name='subcat'><option value=''>Model</option>";
	while($noticia = mysql_fetch_array($quer)) { 
		echo  "<option value='$noticia[subcategory]'>$noticia[subcategory]</option>";
		$subcat = $noticia[subcategory];
			echo "</select>";
			echo "<br>";
			echo "<img src="/xxxxx/images/spacer_4x4.gif">";
			echo "<br>";
//Display Second Pull Down Form End

//Display Submit Button Begin
echo "<input name="submitted" type="submit" value="Go!">";
//Display Submit Button End
echo "</form>";
//Form End
//Center All Elements <DIV> End
echo "</div>";

First of all: mysql_escape_string() is deprecated afaik. Use mysql_real_escape_string() instead. Also: ALWAYS assume register_globals to be turned OFF. Reason for this is that register_globals allows for uninitialized variables to contain malicious user input, which poses a security issue. Always use $_GET, $_POST, etc (since $HTTP_GET_VARS, etc. is also deprecated). Also ALWAYS initialize variables, i.e. assign them a solid value. Even if it’s just $myVar = “”; or $myInt = 0; What you need to keep in mind while programming is that ANY and ALL user input can contain malicious values. Check the variables containing user input ($_GET, $_POST, $_COOKIE) thoroughly to make sure there are no unwanted characters in there.

Sponsor our Newsletter | Privacy Policy | Terms of Service