1

I am having a problem getting a query to work (and also am questioning the security of the query).

  if(isset($_POST)){
        $sql = "SELECT * FROM members WHERE";

     if($_POST['FirstName_S'] !== ''){
        $sql .= " FirstName LIKE '%" . $_POST['FirstName_S'] . "%'";
     }
     if($_POST['LastName_S'] !== ''){
        $sql .= " OR LastName LIKE '%" . $_POST['LastName_S'] . "%'";
     }
     if($_POST['Firm_S'] !== ''){
        $sql .= " OR Firm LIKE '%" . $_POST['Firm_S'] . "%'";
     }
     if($_POST['Country_S'] !== ''){
        $sql .= " OR Country LIKE '%" . $_POST['Country_S'] . "%'";
     }
     if($_POST['City_S'] !== ''){
        $sql .= " OR City LIKE '%" . $_POST['City_S'] . "%'";
     }
     if($_POST['State_S'] !== '' AND $_POST['State_S'] !== 'other'){
        $sql .= " OR State LIKE '%" . $_POST['State_S'] . "%'";
     }
  }

Obviously, if FirstName_S is undefined, the query breaks saying "WHERE OR". It seems like it would have a logical fix, but I've been staring at it for a little too long.

Also, sql injection was brought up as a concern, and as a side-question, would sanitizing the input be enough? Or is this altogether bad practice?

aceslowman
  • 621
  • 13
  • 28
  • Is `FirstName_S` required to be sent in the `POST`? – emco Aug 12 '13 at 17:18
  • no, it's not. It's just a search function where users can look up members in a database. You can just enter any one of these alone, or with others. – aceslowman Aug 12 '13 at 17:20
  • 3
    To protect from SQL Injections, prepared statements are best: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php/60496#60496 – General_Twyckenham Aug 12 '13 at 17:20
  • Move the `OR` from the beginning to the end of the line – Nir Alfasi Aug 12 '13 at 17:22
  • 1
    Just end the query with `WHERE 1=1`. Then you can append any number of `OR something` without being concerned about omitting the `OR` from the first one. (There are some query builder tools that use this technique.) If you are using mysql interface, yes, sanitize the post variables. If you are using mysqli or PDO (and you should be) use parameterized queries. See my answer. – spencer7593 Aug 12 '13 at 17:44

6 Answers6

3
  if(isset($_POST)){
        $sql = "SELECT * FROM members WHERE";

     if($_POST['FirstName_S'] !== ''){
        $sql .= "OR FirstName LIKE '%" . $_POST['FirstName_S'] . "%'";
     }
     if($_POST['LastName_S'] !== ''){
        $sql .= " OR LastName LIKE '%" . $_POST['LastName_S'] . "%'";
     }
     if($_POST['Firm_S'] !== ''){
        $sql .= " OR Firm LIKE '%" . $_POST['Firm_S'] . "%'";
     }
     if($_POST['Country_S'] !== ''){
        $sql .= " OR Country LIKE '%" . $_POST['Country_S'] . "%'";
     }
     if($_POST['City_S'] !== ''){
        $sql .= " OR City LIKE '%" . $_POST['City_S'] . "%'";
     }
     if($_POST['State_S'] !== '' AND $_POST['State_S'] !== 'other'){
        $sql .= " OR State LIKE '%" . $_POST['State_S'] . "%'";
     }

    $sql=str_replace("WHERE OR","WHERE",$sql);   // quick dirty fix

  }

Ofcourse you'd need to sanitize the input, but since you didn't mention which MySQL API you use, I did not add any sanitization functions yet. You can look at http://php.net/mysqli_real_escape_string

Hanky Panky
  • 46,730
  • 8
  • 72
  • 95
2

do it other way as follow

if(isset($_POST)){
        $sql = "SELECT * FROM members WHERE";

     if($_POST['FirstName_S'] !== ''){
        $sql_arr[]=" FirstName LIKE '%" . $_POST['FirstName_S'] . "%'";

     }
     if($_POST['LastName_S'] !== ''){
        $sql_arr[]= " LastName LIKE '%" . $_POST['LastName_S'] . "%'";
     }
     if($_POST['Firm_S'] !== ''){
        $sql_arr[]= " Firm LIKE '%" . $_POST['Firm_S'] . "%'";
     }
     if($_POST['Country_S'] !== ''){
        $sql_arr[]= " Country LIKE '%" . $_POST['Country_S'] . "%'";
     }
     if($_POST['City_S'] !== ''){
        $sql_arr[]= " City LIKE '%" . $_POST['City_S'] . "%'";
     }
     if($_POST['State_S'] !== '' AND $_POST['State_S'] !== 'other'){
        $sql_arr[]= " State LIKE '%" . $_POST['State_S'] . "%'";
     }
     if(!empty($sql_arr)){
       $sql.=implode(' OR ',$sql_arr);
}

  }
Garry
  • 595
  • 4
  • 19
2

The quick fix is to add 1=1 to your query, so your query ends with WHERE 1=1. That allows you to be free to append any number of OR something to your query, without being concerned with omitting the OR on the first one.

(That 1=1 predicate doesn't cause any problem; that will be evaluated at parse time, and doesn't appear in the execution plan.)

As to SQL Injection, yes, this code is susceptible. If you are using mysql interface, then sanitize the post variables with mysql_real_escape_string function. If you are using mysqli or PDO (and you should be), then use parameterized queries.

spencer7593
  • 106,611
  • 15
  • 112
  • 140
1
$stmt = $dbConnection->prepare('SELECT * FROM members WHERE FirstName LIKE ? OR LastName LIKE ? OR FIRM LIKE ? OR Country LIKE ? OR CITY LIKE ? OR STATE LIKE ?');


if($_POST['FirstName_S'] !== ''){
  $stmt->bind_param('FirstName', '%'.$_POST['FirstName_S'].'%');
} else {
  $stmt->bind_param('FirstName', '%');
}

… // do this for all your parameters

$stmt->execute();
Mirco Widmer
  • 2,139
  • 1
  • 20
  • 44
0

I think this could help you:

if(isset($_POST)){
    $sql = "SELECT * FROM members";

 if($_POST['FirstName_S'] !== ''){
    $sql .= " WHERE FirstName LIKE '%" . $_POST['FirstName_S'] . "%'";
 }
 else {
   $sql .= " WHERE FirstName LIKE '%'";
 }
 if($_POST['LastName_S'] !== ''){
    $sql .= " OR LastName LIKE '%" . $_POST['LastName_S'] . "%'";
 }
 if($_POST['Firm_S'] !== ''){
    $sql .= " OR Firm LIKE '%" . $_POST['Firm_S'] . "%'";
 }
 if($_POST['Country_S'] !== ''){
    $sql .= " OR Country LIKE '%" . $_POST['Country_S'] . "%'";
 }
 if($_POST['City_S'] !== ''){
    $sql .= " OR City LIKE '%" . $_POST['City_S'] . "%'";
 }
 if($_POST['State_S'] !== '' AND $_POST['State_S'] !== 'other'){
    $sql .= " OR State LIKE '%" . $_POST['State_S'] . "%'";
 }

}

and for the SQL injections, you can check General_Twyckenham comment.

emco
  • 4,589
  • 3
  • 18
  • 20
0

You could compose the WHERE command based on what parameters is entered...

if(isset($_POST)){
    $sql_where = '';
    $sql = "SELECT * FROM members ";

    if($_POST['FirstName_S'] !== ''){
        $sql_where .= (($sql_where != '')?('OR '):(''))." FirstName LIKE '%" . $_POST['FirstName_S'] . "%' ";
    }
    if($_POST['LastName_S'] !== ''){
        $sql_where .= (($sql_where != '')?('OR '):(''))." LastName LIKE '%" . $_POST['LastName_S'] . "%' ";
    }
    if($_POST['Firm_S'] !== ''){
        $sql_where .= (($sql_where != '')?('OR '):(''))." Firm LIKE '%" . $_POST['Firm_S'] . "%' ";
    }
    if($_POST['Country_S'] !== ''){
        $sql_where .= (($sql_where != '')?('OR '):(''))." Country LIKE '%" . $_POST['Country_S'] . "%' ";
    }
    if($_POST['City_S'] !== ''){
        $sql_where .= (($sql_where != '')?('OR '):(''))." City LIKE '%" . $_POST['City_S'] . "%' ";
    }
    if($_POST['State_S'] !== '' AND $_POST['State_S'] !== 'other'){
        $sql_where .= (($sql_where != '')?('OR '):(''))." State LIKE '%" . $_POST['State_S'] . "%' ";
    }
    $sql .= (($sql_where != '')?('WHERE '.sql_where):(''));
}