Search code examples
phpsqlcode-injectionsql-delete

Is this code sql injectable? deleting users from within the $_GET url


Sorry, I asked a similar question before related to this but I fixed up the code (I think) to make it full proof against the admin AND! regular user accounts.

The admin and users share the member list page together. The difference is: The admin gets things like

Edit | Delete | Make Admin

Here is the basic code you need to look at most....

if(@$_GET['id'] != ""){
    if (has_access($session_user_id, 1) === false) {
        header('Location: index.php');
        exit();
    }
    $userID = intval($_GET['id']);
        if (!$userID) {
        header('Location: index.php');
        die();
    }
    $sql = "DELETE FROM users WHERE user_id='".$userID."'";
    $query  = mysql_query($sql);
    header('Location: members.php?id=&page=1');
}

Let me explain the different sections of the code:

if(@$_GET['id'] != ""){
    if (has_access($session_user_id, 1) === false) {
    header('Location: index.php');
    exit();
}
.....continued

This says, if the id in the url is NOT equal to "" check the users rank (admin or regular) If he is regular move him to the index page AND STOP THE SCRIPT!

regular users arent supposed to edit the id statement... if they do... it means they are doing so in a goal of hacking or some sort....

Now onto the next part

$userID = intval($_GET['id']);
    if (!$userID) {
    header('Location: index.php');
    die();
}

This basicaly stated intval so the id is ONLY! A NUMBER, this blocks things like

'; DELETE FROM users WHERE user_id=19;

NEXT... an extra security feature, if NOT userid redirect and exit

FINNALLY,

$sql = "DELETE FROM users WHERE user_id='".$userID."'";
    $query  = mysql_query($sql);
    header('Location: members.php?id=&page=1');
}

run the sql and redirect the user back to the same page for refreshed results.

Is this correct logic I am stating?

Here is my full members.php file

http://pastebin.com/tkstyrWg

Also the website is up for testing at

http://1334.3owl.com

The members.php actually has a

protect();

function which will redirect the user to the main page if they are not logged in

You can feel free to test the sql injection on this website...

Here is a demo user you can use to test

demouser
demopass

If the script is not secure... How do I make it so?

Also is it secure against admins?

Is my sql script in the url correct?


Solution

  • I did not check your entire script but in regards to intval, it should be all you need to make it secure as it's only a user id....