0

I've recently been working on remediating security vulnerabilities for a PHP legacy application. We've refactored all MYSQLI queries to use prepared statements as such. This has mostly been straightforward, but I've run into a few isolated situations in which I need to use placeholders (?) for a WHERE clause that is dynamically generated, and can have different elements depending on user input. I'm having trouble finding documentation that shows how to achieve this, so I may need to change my approach.

I'm in a position where I will not know which items need to be added to the where clause until the user selects input. What is considered the best approach to solve this problem?

The original, functioning query works like so:

$query ="SELECT [fields] FROM table1, table2 where (table1.id = table2.cc_id) $where order by num, name, due_date";

$where is a series of dynamic AND statements. I'm only aware of the number of AND clauses/values when the page runs. Example:

AND table1.business_line_id IN (2) AND table1.client_id IN (1,2,3,5) AND table1.application_id in (7,47) AND table1.region_id in (1,4,5,6,8,14,15,21)

The rest of the code related to this item:

$params = array($where);

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
if(!$stmt = $connection->prepare($query)) {
    showerror($connection);
    die();
}
// $stmt -> bind_param(get_types($params),...$params);
$stmt -> bind_param('s', $params);
$stmt -> execute();

The array of parameters returns an array of length 1 with the correct $where value of type string. I've tried to implement a placeholder, but this does not traditionally follow the structure of placeholders in which we use field = placeholder (user_id = ?) or VALUES(?,?,?). Is it possible to generate a query in which we use placeholders dynamically generated sections of the WHERE/AND clauses with this syntax?

Here's an example of something that does not work.

    // $query ="SELECT [fields] from table1, table2 WHERE (table1.id = table2.cc_id) ? order by num, name, due_date";

Understandably, MYSQLI's $stmt = $connection->prepare($query) takes issue with this at the placeholder value (?). If I try to include the $where variable directly, the bind_param() call takes issue because it has no parameters to bind (number of variables doesn't match).

Here's an example that does work, but I'm concerned it won't properly protect against SQL injection. Just passing $where and removing any parameter binding will give me the proper output.

$query ="SELECT [fields] FROM table1, table2 where (table1.id = table2.cc_id) $where order by num, name, due_date";
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
if(!$stmt = $connection->prepare($query)) {
        // added this during debug to show the malformed query
    showerror($connection);
    die();
}

$stmt -> execute();
if (!$result = $stmt->get_result()) showerror($connection);

Is there a way to pass in this clause using placeholders?

EDIT: adding $where build logic for clarity.

$where = "and cc.business_line_id in ($business_line_ids)";
// some code
foreach($ar_select as $value) {
    if (isset($_POST["${value}_list"])) {
        $thisfield = ($value == "assignee_status_id") ? "table2.status_id" : "cc.$value";
        $this_id_list = implode(",", $_POST["${value}_list"]);
        $where .= " AND $thisfield in ($this_id_list) ";
    }
}
// some code
$ar_dates = array("status_date","target_date","due_date");
foreach($ar_dates as $thisdate) {
    if (strlen($_POST["$thisdate"])) {
        $date = $_POST["$thisdate"];
        $op_date = $_POST["op_$thisdate"];
        $where .= " AND $thisdate " . date_operator($op_date) ." '$date' ";
    }
}

if (strlen($_POST["keyword_name"])) {
    $keyword_name = $_POST["keyword_name"];
    $where .= " AND asn.name LIKE '%$keyword_name%' ";
}
kdft02
  • 11
  • 1
  • 1
    How you construct the `$query` value is no different then constructing any string. You can concatenate strings, interpolate variables in strings, etc. As long as the strings are not being built directly from user-controlled values and only values *you* control, it makes no difference. SQL injection only occurs at the point at which that SQL code is *executed*. As long as the *resulting* query uses value placeholders and executing that query uses parameters, it's no different than if that string had been created statically. – David Jul 12 '23 at 19:02
  • The key really is how the `$where` string is constructed, which we can't see. That needs to be built up gradually, presumably based on some user input values? It needs to ensure that ?s are used in the right place, and for every ?, a value is added to a separate array of values, which is then added to the parameter list when the query runs. – ADyson Jul 12 '23 at 19:04
  • @ADyson this makes sense, I've added all elements of building $where on the page, part of which comes from $_POST. It sounds like what I need to do is separate these values out into an array with multiple indices so I can explicitly use bind params on each value. Moreover, I found this article (https://stackoverflow.com/questions/2138825/using-prepared-mysqli-statements-to-bind-parameters-into-the-select-section-of-a?rq=2) stating that I can't pass in anything but values with placeholders. It seems I'll need to refactor how $where is built to get this working and account for SQL injection. – kdft02 Jul 12 '23 at 19:11
  • 2
    This is much easier to do using PDO than mysqli, because you can put the parameter values in an array and pass that to `$stmt->execute()`. mysqli has added that capability in PHP 8.0, but if you're using an older version it's harder. – Barmar Jul 12 '23 at 19:22
  • See https://stackoverflow.com/a/28909923/1491895 for how to do it with PDO. – Barmar Jul 12 '23 at 19:22
  • 1
    `It sounds like what I need to do is separate these values out into an array with multiple indices so I can explicitly use bind params on each value`. Yes, basically. For example you've got `$_POST["${value}_list"]` so you can add a separate ? for each item in that into the string, and put the values in a separate list for binding. As Barmar says, this is a lot easier with PHP 8, or with PDO – ADyson Jul 12 '23 at 19:25
  • Thanks to all for the input. Sadly we are working with PHP 7.4 currently I'm unsure how large of a rework moving to PDO would entail. I'll probably try to sort this out myself in the current environment and then look to PDO for the future. – kdft02 Jul 12 '23 at 19:59
  • @ADyson if you put this in as an answer, I'll be glad to accept it. – kdft02 Jul 12 '23 at 20:18
  • @ADyson with mysqli pre-8.0 it's only *two extra lines* of code. Compared to the rest of the code for the task it's just NOTHING, least to be called "much harder". And even these two extra lines can EASILY, in JUST 5 minutes, be encapsulated into a userland function and therefore made it SIMPLER than PDO. – Your Common Sense Jul 13 '23 at 05:59

1 Answers1

0

You can only parameterize values, not statements.

Basically, the SQL interpreter compiles the SQL statement, then slots in the parameter values. At that point you can't re-invoke the compilation to figure out what's inside the parameters, that would invalidate the previous compile and make parameterization pointless. We want to compile the statement once, and interpolate parameters never.

That said, you can modify the way that you construct the statements to include a parameter stack, and just push things on as you go.

Here is a simplified/contrived example:

$params = [];
$clauses = [];

$input = [
    ['foo = ?', ['fooval']],
    ['bar BETWEEN ? AND ?', ['start', 'end']],
    ['baz = 42', []],
    ['bof IS NOT ?', ['notbof']]
];

foreach($input as $item) {
    $clauses[] = $item[0];
    $params = array_merge($params, $item[1]);
}

$where = sprintf('WHERE %s', implode(' AND ', $clauses));

var_dump($where, $params);

Output:

string(67) "WHERE foo = ? AND bar BETWEEN ? AND ? AND baz = 42 AND bof IS NOT ?"
array(4) {
  [0]=>
  string(6) "fooval"
  [1]=>
  string(5) "start"
  [2]=>
  string(3) "end"
  [3]=>
  string(6) "notbof"
}
Sammitch
  • 30,782
  • 7
  • 50
  • 77
  • This code has a weak point with $input. a very weak point. Which, I would say, defeats the very purpose of the whole affair. – Your Common Sense Jul 13 '23 at 05:26
  • Why writing "simplified" answers? In the "simplified" form, the OP personally got their answer already in the comments. While a full featured answer should never be "simplified" as every answer has a double purpose: to help the OP and to help everyone coming later. And for sake of these people the answer must be solid and elaborate, not "simplified" – Your Common Sense Jul 13 '23 at 05:42