CODEWITHSUNDEEP
phpsecurity
daninthemix
daninthemix

Reputation: 2570

Broken PHP page security

I have this at the top of a navigation file which is included on every single page:

if (!is_logged_in()){
        login_error_redirect();
}

Here's that function:

function is_logged_in(){
    if(isset($_SESSION['GBuser']) && $_SESSION['GBuser'] > 0){
        return true;
    }
        return false;
}

Now this works insomuch as if you try to browse to a protected page you get redirected to the login page, but for some reason it doesn't apply if you supply a GET request, which allows you to completely bypass the whole thing. For instance, having just logged out (which calls session_destroy();), I can enter the following address and the item gets deleted:

../site/products.php?delete=20

What am I missing here? That products page includes the same navigation file with the security check above, but passing in a GET variable skips it completely for some reason.

EDIT: here's the top of products.php:

require_once $_SERVER['DOCUMENT_ROOT'].'/shopping/core/init.php';
include 'includes/head.php';
include 'includes/navigation.php';

//if the delete product button was clicked
if(isset($_GET['delete'])){
    $delete_id = (int)($_GET['delete']);
    $db->query("UPDATE products SET deleted = 1 WHERE id = '$delete_id'");
    header('Location: products.php');
}

And at the very top of navigation.php is the check:

if (!is_logged_in()){
        login_error_redirect();
}

Upvotes: 1

Views: 69

Answers (1)

Err
Err

Reputation: 920

Assuming login_error_redirect() actually redirects, you will have to add exit to stop the script after that function is called. By default, PHP will run all your code on the page regardless if you redirect at the top.

if (!is_logged_in()){
        login_error_redirect();
        exit;
}

Upvotes: 5

Related Questions