login security issue

[swatisonee]

i just discovered a hole in my scripts relating to access .

 

1. have a simple login form

2. based on the type of user , he is directed to a page for his options .

3. I now realise that altho each page therefter checks for sessions of the user , he can easily  change the url to that of another user and there is no way to prevent it.

4. How can i make sure that each time a page is accessed it is only by the user whom it is meant for.

 

 

Relevant code snippets below . Thanks ! Swati

 

login.php

---------

 

<?php
//error_reporting(E_ERROR | E_PARSE | E_CORE_ERROR);

//Process this if statement only if form was submitted
if($_POST['submit']){
session_start();

$username=$_POST['username'];
$password=$_POST['password'];
include ("link.php"); // contains db info


    //Test for login success
    $sql = "SELECT * FROM Users WHERE Username='$username' AND Password = '$password'";
    $result = mysql_query($sql);
    if  ($myrow = mysql_fetch_assoc($result)){
//  echo $sql;
       
        $login_success = 'Yes'; 
        $userid = $myrow["Userid"];
       $usertype = $myrow["UTID"];
        
        $status = "On";
        $url = $PHP_SELF."?".$_SERVER['QUERY_STRING'];; 
        $logout = 'logout.php';

        $_SESSION['id']=session_id();
        $_SESSION['userid']=$userid;
        $_SESSION['usertype']=$usertype;


       $sql2= "insert into Log (Sessionid,Userid,IP,Date,File, Status) values('$_SESSION[id]','$userid','$ip','$tm', '$url', '$status')";
$result2 = mysql_query($sql2) or die  ('no access to database: ' . mysql_error());
//        echo mysql_error();      }

}
}      
?>

 

Each subsequent page has this header

==============================

 

 
<?
header("Cache-Control: public");

include ("log.php"); //db info for DB along with session start
if(!isset($_SESSION['userid'])){
echo "<center><font face='Calibri' size='2' color=red>Sorry, Please login and use this page </font></center>";
exit;}
?>

 

 

The url of each page :

 

 
www.abc.com/example/type1.php?Userid=USER1ID

 

and such a user can easily change the url to 

 
www.abc.com/example/type2.php?Userid=USER1ID 

and access all the options of type2.php

[JakeTheSnake3.0]

I'm not sure I'm getting this...you have a separate php page for each different user?  Why?  Why not use a single page which only presents info specific to the userID?

 

What you're doing:

.../page1.php?userID=1010 < page1.php is meant for user 1010

.../page2.php?userID=1010 < page2.php is NOT meant for user 1010???

 

What does page1 and page2 do?  If it's something like a user's details, the info should be generated by the userID...so there should only be one page...like .../page.php?userID=1010 or .../page.php?userID=2011

[swatisonee]

HI :

 

The login page does direct a user to his own access page depend on usertype. For example : sales and finance. so a sales guy would go to sales.php and make reports etc  and the fiance guy would go to finance.php and do his stuff there.

 

the problem arises afterwards.

 

is salesguy knows that finance.php leads to the finance options, he can simply edit the address bar and swap www.abc.com/example/sales.php?Userid1 to www.abc.com/example/finance.php?Userid1

 

Now that should not happen right? when he loads finance.php, the script has to check if he is authorised to access finance.php . But the script currently checks only the session id which in any case is valid as salesguy s session is alreadyt registered.

[JakeTheSnake3.0]

You can put in a binary access control string in your user database which will indicate what pages the specific user can access.  Something like "110" for (userDetails.php - yes, finances.php - yes, marketing.php - no).  Have a custom function like 'validateUser()' which accepts the session variable as an argument and validates whether or not the user can access the given page based on their binary access control string.

[swatisonee]

oh..i've no idea what binary string is...i will check. thanks for the input.

[devilinc]

all you have to do is do a session check for the user type at the start of the page after session_start(); and redirect them to appropriate pages....when doing this check for some column name that differentiates these designations.....surely, u have a master table

which stores the sales group and finance group separately having their own id's to the user's table as a foreign key....so just make this check....

 

so at top of page do this:

eg:sales.php

if(logged in user doesnt belong to sales){

//kick the user out to some other page

}

[swatisonee]

yes the users table has the foll fields - userid, usertype, username . So what your saying is that when the foll is run

if 
(
!isset($_SESSION['userid'])
)
{ 
echo Sorry, Please login and use this page "; 
exit;
}

 

i also check usertype as under  ?

 

$_SESSION['userid']=$userid;
$_SESSION['usertype']=$usertype;

if 
(
!isset($_SESSION['userid']) AND ($_SESSION['usertype'] != "type1" )
)
{ 
echo Sorry, Please login and use this page "; 
exit;
}

 

[JakeTheSnake3.0]

Yes, you've got the idea; the reason why I suggested a binary string is in case you have a user who is allowed access to more than one page.  Each digit in the string represents a boolean value of true or false.  YOU will decide which digit placement means what for which login page.

 

EX) If you have three pages...myaccount.php, accounting.php, emailer.php then your binary string will have 3 digits.  The 1st digit will determine whether or not the user can use the myaccount.php page, the 2nd will do the same for accounting.php and likewise for emailer.php with the 3rd digit.  So a string of "101" will mean the user doesn't have access to accounting.php.  A string of "111" means the user has access to all three pages.  A string of "011" means the user doesn't have access to myaccount.php.  Do you see where I'm going with this?

[swatisonee]

Hi:

 

If i understand the concept of binary access correctly, it would mean adding a field into my users table with binary as an attribute . If that is so then it wont solve my problem  i think.

I have several pages and they are accessible to 3 types of users in various combinations (page1 is for types 1&3, page 2 for types 2&3, page 3 for types2,3 so on so forth) .So unless i define on each page the type of user allowed to access that page, the db is still open to manipulation.

 

Here's what i have just done but it doesnt work. Access is still open thru manipulation of the addressbar

 

page7.php

========

<?php
header("Cache-Control: public");

include ("log.php"); //db info for DB2 // log.php code is below

[color=#007700]if 
(
!isset([/color]$_SESSION['userid']) AND ($_SESSION['usertype'] != "3" ) // only usertype 3 can access this page
)
{ 
echo "Sorry, you are either not logged or you are not authorised to view this page"; 
exit;
}

 

log.php

=====

 

<?php
error_reporting(E_ERROR | E_PARSE | E_CORE_ERROR);
session_start();
include ("link.php") ; // db info
$ip=$_SERVER['REMOTE_ADDR'];
$url = $_SERVER['PHP_SELF']."?".$_SERVER['QUERY_STRING'];
$userid = $_POST['userid'];
$sql = "SELECT * FROM Users WHERE Userid= '$_SESSION[userid]'";
$result = mysql_query($sql);
$myrow = mysql_fetch_array($result);
$utid = $myrow["UTID"];
$_SESSION['usertype']=$utid;
$sql2= "insert into Log (Sessionid,Userid,IP,File, Status) values('$_SESSION[id]','$_SESSION[userid]','$ip','$url', 'On')";
$result2 = mysql_query($sql2) or die  ('no access to database: ' . mysql_error());
//echo $sql2;

?> 

 

 

[JakeTheSnake3.0]

Your code looks like a mess - I'm guessing you tried formatting it for this post and it screwed up somehow.  This is what I assume it should be:

 

if (!isset($_SESSION['userid']) && ($_SESSION['usertype'] != 3)) {

First off, you had a comment between two end braces ) //comment )...I'm not sure if because of the newline that would have worked, but not really a good practice.  Secondly, you used "AND" instead of "&&"..."AND" is used in SQL queries, not PHP code.  Make the changes and if it still doesn't work, make sure you echo the variables you're using to make sure that they are set correctly.