0

I have written a function in PHP to check whether a user can access an order on a web application that I'm making.

It checks what account owns a given order ID and if that account isn't the current session account then the idea is to make the user go back to the home page.

Unfortunately it does not seem to be working, where any user can view any order even though the SQL query is definitely correct (I have verified this through a manual SQL query).

Is there a problem in the code that I have written?

<?php
function is_accessible($document, $account) {
    global $dbh;
    $sth = $dbh->prepare("SELECT account FROM orders WHERE order_id = $document");
    $sth->execute();
    $result = $sth->fetchAll();
    if ($result[0]['0'] == $account) {
    return true;
    } 
    else {
     return false;
    }
}
?>

<?php
if (!is_accessible($_GET['id'], $_SESSION['account'])) {
  header("Location: /index.php");
}
?>

<?php
echo $_GET['id'];
10001
echo $_SESSION['account'];
1
?>
Bijan
  • 7,737
  • 18
  • 89
  • 149
Jack
  • 515
  • 1
  • 5
  • 17
  • Can you show how you are calling the function is_accessible? – Masiorama May 12 '15 at 09:08
  • 4
    You're preparing the statement, but injecting the $document value directly into that statement rather than using a bind variable? :( Is your $document value a string or a number? – Mark Baker May 12 '15 at 09:08
  • 3
    **Danger**: You are **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that you need to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin May 12 '15 at 09:09
  • Have you checked what the respective values of `$account§ and `$result[0]['0']` are? – Quentin May 12 '15 at 09:11
  • Is there something in `$_SESSION['account']`? You should probably check `$result[0]['account']` against `$account`. `$result[0]['0']` is undefined and that makes it equal to `NULL`. If `$_SESSION['account']` is also not set then you have a perfect match in the `is_accessible()` function. Use [`print_r()`](http://php.net/manual/en/function.print-r.php) or [`var_dump()`](http://php.net/manual/en/function.var-dump.php) here and there in the code to see what values you have in the variables. – axiac May 12 '15 at 09:13
  • 1
    If you could re-word your question's headline to hint at the specific problem you are having, that might help in attracting (the right people's) attention. – Burki May 12 '15 at 09:13
  • Hi. I've updated the question showing the values (at the end). – Jack May 12 '15 at 09:18
  • Can you also `var_dump` the actual result from your `is_accessible` function? – Erik S May 12 '15 at 09:48
  • A script will continue running after calling header(), because all that does is tell PHP what headers should be sent when they are sent, they're not sent immediately; so you may want to terminate script execution after the header redirect. - `if (!is_accessible($_GET['id'], $_SESSION['account'])) { header("Location: /index.php"); die(); } ` – Mark Baker May 12 '15 at 11:23

0 Answers0