0

I am used to using parameterised statements but in this statement the number of the parameters can change depending on the number of words in a search string.
So "food" = 1 param, "some food" = 2, "lots of food" = 3 etc etc

I have basically set up a query to allow for searching of a website, to improve functionality i have made it possible to search certain words of columns without it have to be a perfect match.

For example, i concat a club name and its city so;
Name: Mint Club,
City: Leeds Col: Mint Club Leeds

Searching: 'Mint Leeds' would normally result in the row not being found. But with this modification it works.

With the unknown number of parameters i basically want to know how to secure my statement as i think its still open to sql injection.

Code Below:

$keystring = $mysqli->real_escape_string($_POST["s"]);
$key = strtoupper($keystring);

$key_arr = explode(" ", $key);
$num_key = count($key_arr);

if($num_key >= 2){
    $where = '';
    foreach($key_arr as $val){
        $where .= "(CONCAT(name,' ',city) LIKE '%".$val."%') AND ";
    }
    $where = substr($where, 0, -4);
    $clubWhere = $where;
}
else{
    $key = "%".$key."%";
    $clubWhere = "(CONCAT(name,' ',city) LIKE '".$key."')";
}

$search_stmt = $mysqli->prepare("SELECT id, name, type AS 'col3', city AS 'col4', 'Club' AS 'table' FROM studentnights_clubs WHERE ".$clubWhere."
                                    UNION
                                    SELECT id, name, description AS 'col3', image AS 'col4', 'Event' AS 'table' FROM studentnights_events WHERE UCASE(name) LIKE ?
                                    UNION
                                    SELECT id, name, '' AS 'col3', '' AS 'col4', 'Genre' AS 'table' FROM studentnights_music WHERE UCASE(name) LIKE ?
                                    ORDER BY
                                        CASE
                                            WHEN name LIKE ? THEN 1
                                            WHEN name LIKE ? THEN 3
                                            ELSE 2
                                        END
                                    LIMIT 10");
$search_stmt->bind_param('ssss', $key, $key, $key, $key);
$search_stmt->execute();
$search_stmt->store_result();
$search_stmt_num = $search_stmt->num_rows;
$search_stmt->bind_result($id, $name, $col3, $col4, $table);
O. Jones
  • 103,626
  • 17
  • 118
  • 172
JParkinson1991
  • 1,256
  • 1
  • 7
  • 17
  • 1
    possible duplicate of [How can I prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – tyh May 02 '14 at 20:17
  • very different scenarios, i understand how to protect against injections, just not in this situation where the parameters can vary to an unknown number. – JParkinson1991 May 02 '14 at 20:19
  • http://stackoverflow.com/questions/20180214/mysqli-bind-unknown-number-of-parameters that might help – Mattt May 02 '14 at 20:24
  • that question does as im doing here really, there is no parameterisation. Thanks for the help though. Im hoping there is way i can change the 'bind_param' statement to be flexible to a change in paramters. – JParkinson1991 May 02 '14 at 20:32
  • Look at the last part of this answer - http://stackoverflow.com/a/15991146/689579. Putting your values in an array, and using `str_repeat(?, count($arr) - 1) . '?';` to build your placeholders. – Sean May 02 '14 at 20:46

2 Answers2

1
  1. it's a pity you choose raw mysqli for the task - most toilsome API ever existed.
  2. for the fulltext search you should really use either dedicated mysql feature or external search engine like Sphinx Search. It will not only relieve you from this toilsome and inadequate code but it will be actually works with real database, not only with test recordset of hundred rows.
  3. if you want to stick to both inadequate mysql search and mysqli api (as, in my experience, PHP users are especially reluctant for the proper tools) then at least use parameters and then dynamical binding.

Something like this (removing all the useless branching),

$where   = '';
$values  = array();
foreach(explode(" ", $key) as $val)
{
    $values[] = "%$val%";
    $where   .= "(CONCAT(name,' ',city) LIKE ?) AND ";
}
$where = substr($where, 0, -4);

then add other values to the $values array

$values[] = $key;
$values[] = $key;
$values[] = $key;
$values[] = $key;

and then use dynamical binding as explained here https://stackoverflow.com/a/17874410/285587

Community
  • 1
  • 1
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
-1

Your $val and $key items are user-supplied search parameters, so you need to make sure there's no way for rubbish in them to get through to the query.

One possibility: Prepare two different SQL statements, and use the one with the appropriate number of bind variables.

Another: sanitize the incoming strings before using them to construct SQL statements. You said they're place names, supposedly like 'newcastle-upon-tyne', 'denver', or 'new york'. They are probably not 'new ; drop table super_users;' for example.

So here's how you might sanitize them.

$val = $mysqli->real_escape_string(preg_replace('[^- A-Za-z0-9]', '', $val ));

This line of code will eliminate all characters from $val except alphabetics, numerics, space, and dash. Then it will put escapes into the string to sanitize it for mysqli.

If you preprocess 'new ; drop table super_users;' you'll get 'new drop table superusers', which is safe enough to use in a query, although meaningless.

You could also look for forbidden characters in the user-supplied search terms and refuse to do the search if they were present.

Look, we all know it's much safer to use bind variables, but if you can't you can still sanitize stuff.

O. Jones
  • 103,626
  • 17
  • 118
  • 172
  • thank you so much for the reply, i will use your sanitisation method for $val. Preparing two different statements however would not work as the number of bind variables will never be known, they are dependent on the user carrying out the search. Really the help. Is there a list of forbidden characters i can use to check against? – JParkinson1991 May 02 '14 at 21:58