Jump to content

Recommended Posts

I just discovered that I have a major security flaw with my website.

Anyone who logs in to the website can easily access other users information as well as delete and edit other users information just by changing the ID variable in the address bar.

I have user ID Session started on these pages but still people can do anything they like with other users information just by editing the address bar.

For example if your logged in in the address bar of www.mywebsite.com/delete_mystuff.php?id=5 and change the "5" say to a "9" then you will have access to user#9 information.

 

Every important page that I have has this code:

 session_start();

if (!isset($_SESSION['user_id'])) {
// Start defining the URL.
          $url = 'http://' . $_SERVER['HTTP_HOST'] . dirname($_SERVER['PHP_SELF']);
// Check for a trailing slash.
          if ((substr($url, -1) == '/') OR (substr($url, -1) == '\\') ) {
          $url = substr ($url, 0, -1); // Chop off the slash.
}
// Add the page.
$url .= '/index.php';
ob_end_clean(); // Delete the buffer.
header("Location: $url");
exit(); // Quit the script.
} else {
                //Else If Logged In Run The Script

if((isset($_GET['id'])) && (is_numeric($_GET['id']))) {
          $id = (int) $_GET['id'];
} elseif ((isset($_POST['id'])) && (is_numeric($_POST['id']))) {
          $id = (int) $_POST['id'];
} else {
          echo ' No valid ID found, passed in url or form element';
         
          exit();
}


 

What am I doing wrong?

Please help if you know how to correct this.

 

Many thanks in advance.

I am thinking maybe my queries have to be updated to something like adding "WHERE session_Id=".

Is this something I can add to my queries so only those who are logged in can only access their pages no matter if they try and go around it by editing the address bar to a different ID?

 

 

Looks like your using the get method when a user logs in???

 

Dont do this, you need to rewrite your code using the POST method, select from your database where the username and password match, then set a session id.

When you use the get method, the variables are stored in the url and can be changed by the user.

For something like a profile page, you would compare the $_SESSION['user_id'] with the requested id in $_GET['id']. If the requested profile id does not match the current logged in member, your page would only display the 'public' parts of requested profile information. If the requested profile id does match the current logged in member, your page would additionally present menu choices/links that allow the member to view all the available information and edit his information.

Thank you for all your help.

I tried to understand what some of you have suggested, and tried changinging a few things as suggested but still don't have it working the way it should.

 

Please let me explain my situation a little bit more in detail.

 

When someone logs into their account everything is working correctly and as it should. They can see, edit and delete whatever information they like of theirs. The issue is when a user has the option of changing one of multiple similar items in a database.

 

Lets say that the user has 5 different widgets that has various information about it and he would like to edit the description for one of them.

He navigate to a page that lists all of his five widgets that he has on file. He proceeds to choose the widget that he would like to edit (lets say the id number is 3).

When he clicks on a link besides the widget that he wants to edit,  the address bar would show http://www.mywebsite.com/edit_record.php?id=3

Everything is as it should be. He makes his corrections and submits the form. The database is updated correctly as it should.

 

But here is the problem. If this person has intension of malice, when the address bar shows http://www.mywebsite.com/edit_record.php?id=3 and he changes the "3" to "67" and presses enter button, he in effect now has complete control of item #"67" which does not belong to him but another user. And he can successfully change whatever information he wants on item "67".

 

Please excuse my ignorance if this seems simple to you to resolve but not for me.

I am just trying to understand and learn why this is happening and how to prevent it.

 

The answer is simple, you test on every page request if the currently logged in member has permission to access the requested information.

 

How you test if he has permission to access the requested information depends on if that information changes during one visit to your site. If the information for any one member is static and won't change for the duration of one visit to your site, you can retrieve the list (array) of permitted 'id' once at the start of the visit and store them (as an array) in a session variable. If the information for any one member will change during one visit to your site due to actions he takes on any page request, it would be better and simpler to query on every page request if he has permission to access the requested id.

Looks like your using the get method when a user logs in???

 

Dont do this, you need to rewrite your code using the POST method, select from your database where the username and password match, then set a session id.

When you use the get method, the variables are stored in the url and can be changed by the user.

 

There is absolutely no difference with respect to security between GET and POST data. I can easily view and modify POST data. The only thing POST does obfuscate the values from the run of the mill users. But, it is the users that really know what they are doing that you have to worry about. But, as stated by PFMaBiSmAd, the values should be stored in session data.

I am thinking maybe my queries have to be updated to something like adding "WHERE session_Id=".

 

Yes, you could do that, but that is too late in the process. You should not even get to the point of unnecessarily executing an update/delete query for an incorrect user. You should 'short-circuit' the process as early as possible, at a single/common point in your code, to prevent unnecessarily running the remainder of the code/queries on your page.

 

Also, by actually testing if the current member has permission to access the requested information, when he does not, you can output a message and/or log who is attempting to do this. If you just add a condition into the WHERE clause in all the relevant queries, all that will do is prevent the query from altering the information, with no way of producing a message or logging who is doing it.

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.