Building session array from post to build search query

Question

So I am making post requests to a page for search queries. I am turning them into a session array so I can paginate the results. The process kind of feels ugly and long winded. I need code review for two things.

1. Shortening / making this code more elegant if possible
2. Security - any glaringly obvious vulnerabilities I have missed. I am running the posts through mysqli_real_escape_string but is the way I am building the query vulnerable?

input appreciated

<?php
if ($_SERVER['REQUEST_METHOD'] == "POST") {
    //only accept posts from our domain
    if (strpos($_SERVER['HTTP_REFERER'], DOMAIN)) {
        if (isset($_POST['csrf_token']) && $_POST['csrf_token'] === $_SESSION['csrf_token']) {

            // some arrays to check against
            $stock_type         = array(
                "beers",
                "wines",
                "spirits",
                "apops"
            );
            $trade_type         = array(
                "buying",
                "selling"
            );

            // create session array from post variables
            $_SESSION['search'] = array(
                'stock_type' => mysqli_real_escape_string($connection, $_POST['stock_type']),
                'trade_type' => mysqli_real_escape_string($connection, $_POST['trade_type']),
                'brand' => mysqli_real_escape_string($connection, $_POST['brand']),
                'country' => mysqli_real_escape_string($connection, $_POST['country']),
                'min' => mysqli_real_escape_string($connection, $_POST['min']),
                'max' => mysqli_real_escape_string($connection, $_POST['max']),
                'user' => mysqli_real_escape_string($connection, $_POST['user'])
            );

            // the start of the search variable
            $where              = "WHERE t.published = 1";

            // buld up the search variable based on whats been posted
            if (!empty($_SESSION['search']['stock_type']) && in_array($_SESSION['search']['stock_type'], $stock_type)) {
                $where .= " and t.stock_type = '" . $_SESSION['search']['stock_type'] . "'";
            }
            if (!empty($_SESSION['search']['trade_type']) && in_array($_SESSION['search']['trade_type'], $trade_type)) {
                $where .= " and t.buying_selling= '" . $_SESSION['search']['trade_type'] . "'";
            }

            if (!empty($_SESSION['search']['brand'])) {
                $where .= " and t.brand LIKE '%" .$_SESSION['search']['brand']. "%'";
            }

            if (!empty($_SESSION['search']['country']) && in_array($_SESSION['search']['country'], $country_list)) {
                $where .= " and u.country= '" . $_SESSION['search']['country'] . "'";
            }
            if (!empty($_SESSION['search']['min']) && ctype_digit($_SESSION['search']['min'])) {
                $where .= " and t.price >= '" . $_SESSION['search']['min'] . "'";
            }
            if (!empty($_SESSION['search']['max']) && ctype_digit($_SESSION['search']['max'])) {
                $where .= " and t.price <= '" . $_SESSION['search']['max'] . "'";
            }
            if (!empty($_SESSION['search']['user']) && ctype_digit($_SESSION['search']['user'])) {
                $where .= " and u.subscription = '" . $_SESSION['search']['user'] . "'";
            }
            $_SESSION['where'] = $where;
        }
    }
} else {
    $where = $_SESSION['where'];
}
if (isset($_SESSION['where'])) {
    mysqli_select_db($connection, $database_connection);
    $query = "SELECT SQL_CALC_FOUND_ROWS t.*,
                    u.subType,
                    Count(l.trade_id) AS leads 
                    FROM   trading t 
                           LEFT JOIN leads l 
                                  ON l.trade_id = t.trade_id
                          LEFT JOIN users u 
                                  ON u.user_id = t.user_id 
                    $where
                    GROUP  BY t.trade_id 
                    ORDER  BY timestamp DESC 
                    LIMIT  $start, $perpage";
    $something = mysqli_query($connection, $query) or die(mysqli_error($connection));
    $totalRows = mysqli_num_rows($something);
}
?>

Answer 1

Security - any glaringly obvious vulnerabilities I have missed. I am running the posts through mysqli_real_escape_string but is the way I am building the query vulnerable?

Maybe. You didn't post where $start and $perpage come from, if they are user-supplied you may be vulnerable.

Also, you should switch to using prepared statements. Escaping is a weaker form of protection against SQL injections.

Returning early

You can avoid nested if statements by reversing it and returning early:

if ($_SERVER['REQUEST_METHOD'] != "POST") {
    // handle this case
}


if (!strpos($_SERVER['HTTP_REFERER'], DOMAIN)) {
    // return / die with CSRF error
}

if (!isset($_POST['csrf_token']) || $_POST['csrf_token'] !== $_SESSION['csrf_token']) {
    // return / die with CSRF error
}

Storing data in session

$_SESSION isn't really the right place to store temporary data like this, as it is global and only meant to store data across requests.

I think that you are doing this because it makes it easier to see which values are safe, and which are not. If you don't want to switch to prepared statements (which you should), you should use some other means of doing this, such as storing the values in your own array or appending "safe" to the variable names.

Answer 2

I recently switched from mysqli_ to PDO for the SQL injection reason. I'll give you a very basic example of connecting and searching a row:

<?php
  $host = "localhost";
  $name = "database name";
  $user = "root";
  $pass = "root";

  $variable1 = $_POST['variable1'];
  $variable2 = $_POST['variable2'];


  $db_connection = new PDO('mysql:host='. $host .';dbname=' . $name .';charset=utf8', $user, $pass);


  $database_search = $db_connection->prepare('SELECT * FROM table WHERE column1=:variable1 AND column2=:variable2');
  $database_search->bindValue(':variable1', $variable1, PDO::PARAM_STR);
  $database_search->bindValue(':variable2', $variable2, PDO::PARAM_STR);
  $database_search->execute();
  $database_search_results = $database_search->fetch();

  echo $database_search_results['column1'];

Note that when I use bindValue() on $variable1 and $variable2, the database already knows what the query is, which makes the query invulnerable to SQL injection.

Answer 3

I like the idea of returning early - hadn't thought of this. Below is the full snippet after a tidy up including the $start & $perpage get values. Ended up going with a foreach loop and switch statements.

<?php
if (!isset($_SESSION)) {
    session_start();
}
$perpage = 10;
if (isset($_GET["page"])) {
    if (!ctype_digit($_GET['page'])) {
        exit();
    } else {
        $page = intval($_GET["page"]);
    }
} else {
    $page = 1;
}
$calc  = $perpage * $page;
$start = $calc - $perpage;
if ($_SERVER['REQUEST_METHOD'] == "POST") {
    //only accept posts from our domain
    if (strpos($_SERVER['HTTP_REFERER'], DOMAIN)) {
        if (isset($_POST['csrf_token']) && $_POST['csrf_token'] === $_SESSION['csrf_token']) {
            $ok_values          = array(
                "beers",
                "wines",
                "spirits",
                "apops",
                "buying",
                "selling",
                "1"
            );
            $_SESSION['search'] = array(
                't.stock_type' => $_POST['stock_type'],
                't.buying_selling' => $_POST['trade_type'],
                'brand' => $_POST['brand'],
                'u.country' => $_POST['country'],
                'min' => $_POST['min'],
                'max' => $_POST['max'],
                'u.subscription' => $_POST['user']
            );
            $where = "WHERE t.published = 1";
            foreach ($_SESSION['search'] as $key => $value) {
                if (!empty($value)) {
                    $value = mysqli_real_escape_string($connection, $value);
                    switch ($key) {
                        case "brand":
                            $where .= " and t.brand LIKE '%" . $value . "%'";
                            break;
                        case "min":
                            if (ctype_digit($value)) {
                                $where .= " and t.price >= '" . $value . "'";
                            }
                            break;
                        case "max":
                            if (ctype_digit($value)) {
                                $where .= " and t.price <= '" . $value . "'";
                            }
                            break;
                        default:
                            if (in_array($value, $ok_values)) {
                                $where .= " and $key = '" . $value . "'";
                            }
                    }
                }
            }
            if (isset($where)) {
                $_SESSION['where'] = $where;
            }
        }
    }
} else {
    if (isset($_SESSION['where'])) {
        $where = $_SESSION['where'];
    }
}
mysqli_select_db($connection, $database_connection);
$query = "SELECT    SQL_CALC_FOUND_ROWS t.*, 
                      u.subtype, 
                      count(l.trade_id) AS leads 
            FROM      trading t 
            LEFT JOIN leads l 
            ON        l.trade_id = t.trade_id 
            LEFT JOIN users u 
            ON        u.user_id = t.user_id $where 
            GROUP BY  t.trade_id 
            ORDER BY  timestamp DESC 
            LIMIT     $start, $perpage";
$something = mysqli_query($connection, $query) or die(mysqli_error($connection));
$totalRows = mysqli_num_rows($something);
// total rows before limit clause
$rows      = sprintf("SELECT FOUND_ROWS() AS `found_rows`");
$total = mysqli_query($connection, $rows) or die(mysqli_error($connection));
$rows_total = mysqli_fetch_assoc($total);
?>