1

I am currently switching my site made in Dreamweaver which used a lot of mysql over to mysqli for better security.

my site starts out with a prepare statement:

if (!isset($_GET['orderby']) or ($_GET['orderby'])=="Something") {
    $orderby = "Something Else";
} else {
  $orderby = $_GET['orderby'];
}

if ($stmt = $local->prepare("SELECT * FROM Table ORDER BY ? ASC LIMIT 0,10")) {
    $param = "Table." . $orderby;
    $stmt->bind_param('s', $param);
    $stmt->execute();
    $Recordset1 = $stmt->get_result();
    $row_Recordset1 = $Recordset1->fetch_assoc();
    $stmt->close();
}

this gets called and the table is made on my website. I used to have 4 HREF links above the table that would change the column selected(where the '?' is) and refresh the page(PHP_self) with the new query.

<?php 

echo '<a href='.htmlspecialchars($_SERVER["PHP_SELF"], ENT_QUOTES, "utf-8").'?orderby=Up>Popularity</a>';

?>

Whenever I click on the links now, it adds the "?orderby=Up" to the address but doesn't refresh the query. Am I setting up the prepared statement in the wrong way for this to be accomplished?

Kevin
  • 41,694
  • 12
  • 53
  • 70
Solarplex
  • 107
  • 2
  • 9
  • 1
    FYI, you can't use bound parameters for database identifiers (table / column names, aliases, etc). Your query will end up having the equivalent of `ORDER BY 'Table.Up' ASC` which doesn't mean anything. – Phil Oct 24 '14 at 04:29
  • I know the duplicate is for PDO, not mysqli but the restrictions are the same – Phil Oct 24 '14 at 04:30

1 Answers1

1

Yes, prepared statements is good, but the limitation is:

You cannot bind table/column names.

You can just whitelist them though:

// define your table columns
$columns_on_that_table = array('id', 'name', 'date');
if (isset($_GET['orderby']) && in_array($_GET['orderby'])) {
    $orderby = $_GET['orderby']
} else {
    $orderby = 'id'; // default order column
}

$row_Recordset1 = array();
if ($stmt = $local->prepare(" SELECT * FROM Table ORDER BY `$orderby` ASC LIMIT 0,10 ")) {

    $stmt->execute();
    $Recordset1 = $stmt->get_result();
    // loop them if you want more than one row
    while($row = $Recordset1->fetch_assoc()) {
        $row_Recordset1[] = $row;
    }
    $stmt->close();

}
Kevin
  • 41,694
  • 12
  • 53
  • 70
  • Only thing I had to fix from your code: `if (isset($_GET['orderby']) && in_array($_GET['orderby']**, $columns_on_that_table**)) { $orderby = $_GET['orderby']**;** }` – Solarplex Oct 24 '14 at 05:00