Sql injection prevention adequacy

Hi guys, is the following enough to prevent sql injection? I’ve read other posts on the subject and it seems I’ve done enough. Thanks. :slight_smile:

<?php 
 
 if ($searching =="yes") 
 { 
 echo "<br><br><b>Search Results:</b><br><br>"; 
 
 if ($find == "") 
 { 
 echo "You forgot to enter a search term.<br><br>"; 
 exit; 
 } 

 //$find = strtoupper($find); 
 $find = strip_tags($find); 
 $find = trim ($find);
 
 $find = mysql_real_escape_string($find); 
  
 $result = mysql_query("SELECT * FROM customers WHERE industry='$industry' AND ( company LIKE'%{$find}%' OR email LIKE'%{$find}%' OR website LIKE'%{$find}%' ) ORDER BY company"); 

if(mysql_num_rows($result)==0) {
    echo 'I am sorry your search for <span style="font-weight:bold;color:#336699;">'.$find.'</span> returned no results.<br><br>';
	} else {
	while ($row = mysql_fetch_array($result)) {
      echo '<span class="listing">' . $row['company'] . '</span><br>';
      echo 'Phone: ' . $row['phone'] . '<br>';
      echo 'Address: ' . $row['street'] . '<br>';
      echo 'Email: <a href="mailto:'.$row['email'].'">' . $row['email'] . '</a><br>';
	  echo 'Website: <a href="http://'.$row['website'].'" target="_blank">' . $row['website'] . '</a><br><br>';
      echo $row['description'] . '<br><br><hr><br>';	  
    }
}

 }
?>

$find seems to be sanitized well enough :slight_smile:

What about $industry ?

WHERE industry='$industry'

Where does it come from?

Which brings up what I think is a good point. If you use mysqli_prepare (or PDO) instead of mysql_query then the need for explicitly escaping every input goes away. One less thing to worry about.

Thanks for the replies guys. ‘industry’ comes from a drop down list. It isn’t a input field. Should I do the same to the ‘industry’ value anyways?

Yep. Anything coming in from the outside worlds needs to be sanitized. It’s trivial for someone to change the values of posted information. Of course if you use prepared statements then the problem goes away.

Keep industry as an array.

industries.php


<?php
$industries = array('aviation','mining','drinking');

then include that single array but use it to a) generate your droplist and b) act as a white-list when you filter the incoming $industry from your form.


<?php
include 'industries.php';

// do stuff

if( isset($industry) && !in_array( $industry, $industries)){
// then $industry was set, but had been tampered with
// log this user out or send away etc
}

// else carry on, $industry must be good ...

Got to add a new industry? Add it in one place only. :wink:

That’s great! Thanks a lot guys. :slight_smile: