Passing conditions for filtering a result set from one page to another and back on a safe way

Question

On a php page, I'm displaying users the results of a query against the database. In addition, I give users the possibility to filter these results on 3 different parameters, so to only display the appropriate part from the database. I am wondering if my approach is safe enough, and whether there are easier or safer approaches achieving the same results.

More in-depth: I use the filters provider, client and phone.

One provider can have multiple clients, one client can have multiple phones. The user fills their filters through a <form method='GET'>, resulting in a Query String as follows:

?displayFilters[]=provider|1&displayFilters[]=client|&displayFilters[]=phone|3

In the case above, the user has decided to only display results for provider '1', and for phone type '3'. No filter was set for client, hence after the | is nothing. In my page, I analyse this query string to put the filter values in an appropriate array as follows:

if (isset($_GET['displayFilters'])) {
$displayFiltersTmp = $_GET['displayFilters'];
foreach ($displayFiltersTmp as $value) {
    $displayFilters[substr($value, 0, strpos($value, "|"))] = substr($value, strpos($value, "|")+1);
}
foreach ($displayFilters as $key => $value) {
    if ($displayFiltersURL == "")
        $displayFiltersURL .= "displayFilters".urlencode("[]") . "=" . $key . urlencode("|") . $value;
    else
        $displayFiltersURL .= "&displayFilters".urlencode("[]") . "=" . $key . urlencode("|") . $value;
}
}

This way I can more easily manipulate the MySQL query for filtering the results as asked.

I also regenerate the original Query String for other use. I do this because the results that this page displays can be edited in a seperate page. Through my solution, I can pass these filters as a (couple of) GET-parameter(s), which can then be passed back to the original page displaying the results. Hence, after editing, the user is referred back to the displaying page with their filters still applied.

I am now wondering if this approach is 'safe' (I do the necessary checks before applying the filters to the MySQL-query), but also whether there are easier and/or more elegant solutions to this problem.

What I have now works, but I have concerns considering safety and perhaps stability.

Thanks for anyone willing to look into this! Kenneth Geets

You should rather urlencode `$key` and `$value`, not the `[]` and `|`. I would actually rather use three different query strings, like: `?provider=1&client=2&phone=3` instead of building it and parsing it the way you're doing. It's just unnecessary complexity. Other than that, as long as you're using Prepared Statements when querying the database, I can't really see how it could be regarded as unsafe. Is there something specific you're thinking about? — M. Eriksson, Jan 17 '17 at 15:13
Hi Magnus, the reason I used this approach is that the amount of filters & type of them could change over time; this way I can "easily" include another condition without having to write certain checks. I'm currently not using Prepared Statements; I'm unquoting my queries consequently though. Is there any additional upside to using Prepared Statements? — Kenneth, Jan 17 '17 at 16:04
Prepared Statements are more secure than escaping and concatenating your queries since the data and the SQL-statements are sent to the server separately (the data won't be "inserted" in the SQL statement itself). With PS, you don't even need to think about escaping the data. Regarding using an array vs. separate query params, I don't really see the differens, since you just as easily can loop through the `$_GET`-param and do the exact same thing, but without the parsing of the pipe-sign. You still need to add them to the QS and handle them when querying the database. — M. Eriksson, Jan 17 '17 at 16:19
Another answer, regarding Prepared Statements: http://stackoverflow.com/questions/732561/why-is-using-a-mysql-prepared-statement-more-secure-than-using-the-common-escape (PS is more or less the "de facto"-standard in the PHP-world). — M. Eriksson, Jan 17 '17 at 16:22
Thank you, Magnus. Should I remove my question, or provide it with an answer from myself? Or would you like to phrase something as an answer which I can then accept? — Kenneth, Jan 18 '17 at 08:27

M. Eriksson · Accepted Answer · 2017-01-18T15:11:51.633

Regarding security, the only thing I would change is to urlencode() the $key and $value variables instead of the [] and |-characters. The $key and $value parameters are user inputs and that's always the weakness in any code.

Note: If you are going to display those values for the visitors in HTML, then you should rather use htmlspecialchars(). But since you only are going to build the url again, then urlencode() is enough.

Otherwise, your code should be safe enough, as long as you're using Prepared Statements when you query your database (specially when using user inputs in your queries).

As we discussed in the comments, there are other ways of doing what you are doing, but that's not really an answer to the actual question, so let's leave that in the comments.

Passing conditions for filtering a result set from one page to another and back on a safe way

1 Answers1