php coding exposed to risks

h4r00n

Hi, I have the following code for my search.php and I am aware that it is exposed to risks. I am new to PHP, can anyone help me add some coding to reduce this risk? Thanks

<?php

//get data
$button = (isset($_GET['submit'])) ? $_GET['submit'] : "default_value";
$search = (isset($_GET['search'])) ? $_GET['search'] : "default_value";

if (!$button)
{
   echo "You didn't submit a keyword.";
}
else
{
   if (strlen($search)<=2)
   {
      echo "Search term too short.";
   }
   else
   {
      echo "You searched for <b>$search</b><hr size='1'>";
      //connect to our database
      mysql_connect("localhost","root","");
      mysql_select_db("jobjar");
      //explode our search term
      $search_exploded = explode(" ",$search);

  foreach($search_exploded as $search_each)
  {
     //construct query
	 $x = 0;
$construct = "";
         $x++;
         if ($x==1)            

         {
            $construct .= " Keywords LIKE '%$search_each%'";
         }
         else
         {
            $construct .= " OR Keywords LIKE '%$search_each%'";
         }
      }

  //echo out construct
  $construct = "SELECT * FROM jobs WHERE $construct";
  $run = mysql_query($construct);
  $foundnum = mysql_num_rows($run);

  if ($foundnum==0)
  {
     echo "No jobs found.";
  }
  else
  {
     echo "$foundnum found!<p><br>";

     while ($runrows = mysql_fetch_assoc($run))
     {
        //get data
        $Title = $runrows['Title'];
        $Location = $runrows['Location'];
        $Salary = $runrows['Salary'];
        $Sector = $runrows['Sector'];
        $JobType = $runrows['Job Type'];
        $Duration = $runrows['Duration'];
        $JobRef = $runrows['Job Ref'];
        $Description = $runrows['Description'];

        echo "<b>Title:</b> $Title<br> 
        <b>Location:</b> $Location<br> 
        <b>Salary:</b> $Salary<br>
        <b>Sector:</b> $Sector<br>
        <b>Job Type:</b> $JobType<br>
        <b>Duration:</b> $Duration<br>
        <b>Job Ref:</b> $JobRef<br>
        <b>Description:</b> $Description<p><br>";
     }
  }
   }
}


?>

johanafm

$button will always hold either the value of the input named submit, or the string "default_value". I'm guessing this is a submit button, and that you have given it a value (other than 0), so this will NEVER be true, even if someone reaches the page without submitting the form

if (!$button)

The same line of reasoning goes for $search. If the user inputs nothing, $search will be set to "default_value", and later on you will tell the user they searched for "default_value", and also needlessly query the database

Replace "default_value" with false for $button, and replace it with '' (empty string) for $search. If you want to supply a default string to search for, do so in the value attribute of the text input in the form.

Other than that, always escape strings:

Keywords LIKE '%" . mysql_real_escape_string($search_each) . "%'";

And while you're at it, why not replace the mysql extension with mysqli and prepared statements

h4r00n

Hi,
I kindof understand what you mean. When the user does type anything in the field and just hits search, it states 'no jobs found'. What do you suggest I do?

johanafm

Umm, that depends on wether something should be found or not.

If the user searches for something not in the db, then all is well. If they do search for something in the DB, error_log or echo your SQL statements and have a look at those. If you see nothing weird with it, try executing the exact same query through the DB's CLI. Then compare results...