SQL prepared statement to search songs by various text fields

Question

As we know, using PDO and prepared statements, binding a table or column using bindParam is not possible. So for this, I've figured out to use function parameters to do it by replacing the column names with the parameters, as shown in the example bellow.

function search($filter,$input,$type,$order,$sort,$limit){
    $st=$this->conn->prepare("SELECT Title,Type,Youtube,Score,Ratings,Singer,ID
    FROM song WHERE $filter LIKE CONCAT('%',?,'%')
    AND Type=? ORDER BY $order $sort LIMIT $limit");
    $st->bindparam(1,$input);
    $st->bindparam(2,$type);
    $st->execute();
    return $st->fetchall();
}

And I call this function here:

$result=$search->search($_POST['filter'],$_POST['input'],$_POST['type'],$_POST['order'],$_POST['sort'],$_POST['limit']);

Where the values come from:

• $filter----select
• $input----input
• $type----radio
• $order----select
• $sort----radio
• $limit----input

As you see the table I'm selecting from is fix, that's not optional, but I have the option to choose which column I want the result to be ordered by($order), which column I'd like to search($filter), I want it to be ascending or descending($sort), and how many I'd like to see($limit).

And this code works fine, but I'm not sure if it's a safe way to do this. Are there some kind of risks of this solution, or it's safe to leave it like this?

Answer 1

Are there any risks of using function parameters for column names in prepared statement?

Well, of course you shouldn't. "Function parameters" is not a protection measure in any sensible way. It's just a transport, to deliver some value into a function. As is.

Are there some kind of risks of this solution,

ANY of them. All kinds of SQL injection are welcome in this code.

Sorry for the harsh preface but I wanted to make it clear and make sure there are no such illusions left.

So you have to change the approach entirely. Luckily, it is not hard to implement.

• For the field name and order by, you will need a white list filtering.
• For the limit, as it's not a column name, but a data literal, simply add it through a placeholder.

I've got an article that explains the white list approach in detail, Adding a field name in the ORDER BY clause based on the user's choice

Using a helper function from this article,

function white_list(&$value, $default, $allowed, $message) {
    if (empty($value)) {
        return $default;
    }
    $key = array_search($value, $allowed, true);
    if ($key === false) { 
        throw new InvalidArgumentException($message); 
    }
    return $value;
}

we can make your function 100% safe

function search($filter,$input,$type,$order,$sort,$limit)
{
    $fields = ['name', 'text', 'whatever'];
    $filter = white_list($filter, $fields[0], $fields, "Incorrect filter name");
    $order  = white_list($order, $fields[0], $fields, "Incorrect order name");
    $sort   = white_list($sort, "ASC", ["ASC","DESC"], "Invalid ORDER BY direction");

    $sql = "SELECT Title,Type,Youtube,Score,Ratings,Singer,ID
    FROM song WHERE $filter LIKE CONCAT('%',?,'%')
    AND Type=? ORDER BY $order $sort LIMIT ?"
    $st = $this->conn->prepare($sql);
    $st->execute([$input, $type, $limit]);
    return $st->fetchall();
}

Just edit the list of fields allowed (or make two lists if they have to be different for the filter and order) and you're set.

Answer 2

You should be validating and sanitizing the variables that are not being parameterized.

When there is a strict set of acceptable values, write an array of whitelisted values and check against that.

If input is coming from a text type input or textarea, then it will be harder to validate, and you may have to settle for stripping html or characters which you deem to be out-of-bounds.

You need to be as ruthless as possible without damaging the user experience because people that may attempt to compromise your system will not be pulling any punches.

Some examples:

1. There is only one sort value worth receiving / using in your query. ASC is just syntactic sugar. Because the form provides a radio input field, you don't need to bother with trimming whitespace.

   $sort = $_POST['sort'] == 'DESC' ? 'DESC' : '';
   

2. Unless you are accepting the offset and row count as a single value, the limit value MUST be an integer. Since your form provides a text input field for this, it is possible that a well-intended user may accidentally pass whitespace. For this reason, it would be a sensible inclusion to trim the value before validating it.

   $limit = ctype_digit(trim($_POST['limit'])) ? (int)$_POST['limit'] : '';
   

3. filter and $order come from select input fields. This means you know exactly which values to expect. A lookup array will do the job in a tidy fashion.

   $filterLookup = ['filter1', 'filter2', 'filter3'];
   $filter = in_array($_POST['filter'], $filterLookup) ? $_POST['filter'] : '';
   

   In your project structure, you might even design a single reference point for filter and order column values so that everything stays in sync. You wouldn't want your lookup array to deviate from your form's select options -- that would lead to user frustration.

---

As for how you treat values that do not fall within the predicted range -- that is up to you. You should inform the user when the request could not be processed due to invalid/missing data and let them know how they can fix up their submission. In some cases, you may wish to simply ignore faulty values and proceed with the query construction with that segment omitted -- again, that is your call.

p.s. Consider these adjustments for the sake of readability (while this largely comes down to personal style, it is generally a good idea to write code that doesn't require horizontal scrolling to read):

$result = $search->search(
    $_POST['filter'],
    $_POST['input'],
    $_POST['type'],
    $_POST['order'],
    $_POST['sort'],
    $_POST['limit']
);

and

function search($filter, $input, $type, $order, $sort, $limit) {
    $stmt = $this->conn->prepare(
        "SELECT Title, Type, Youtube, Score, Ratings, Singer, ID
         FROM song
         WHERE $filter LIKE CONCAT('%', ?, '%')
           AND Type = ?
         ORDER BY $order $sort
         LIMIT $limit"
    );
    $stmt->execute([$input, $type]);
    return $stmt->fetchall();
}

---

Regarding your comment about sanitizing limit values, I recommend a min-max sandwich after casting the input as an integer... (Demo)

$inputs = ['7', '54', '3', 'foo'];
foreach ($inputs as $input) {
    echo "---\n$input >>> ";
    $sanitized = min(max((int)$input, 5), 50);
    var_dump($sanitized);
}

Output:

---
7 >>> int(7)
---
54 >>> int(50)
---
3 >>> int(5)
---
foo >>> int(5)