Login Issue - how to secure properly

[bruckerrlb]

I'm having issues with a login form, multiple users use this form, basically I took over the project from the last person and it has a ton of security holes in it, I'm trying to redo the login form so that it doesn't post the userid, right now a link is something like form.php?companyid=4 and I noticed that if I take out that 4 and put in a 3 or any number for that matter, I'm taken to someone elses page, and if I take out the number and just leave the companyid= I see all info, so I need to change that asap, but I'm having some issues with trying to get it set up. I'm trying to change the link to not rely on companyid and on a session, basically my code looks like:

 

<?php



if ($_POST['Email'] && $_POST['Password']) {

session_start();
//db info
include("admin/conf.php");
//open db
include("admin/includes/db_open.php");

$_POST['Email']=trim($_POST['Email']);

$_POST['Password']=trim($_POST['Password']);


//This is how I want to show the company, instead of using their number, I'd rather have a random number stored in the db
$randval=mt_rand();




//store it in the db
mysql_query("UPDATE tbl_company SET sess = '$randval' WHERE Email ='". $_POST['Email'] ."' and Password='". $_POST['Password'] ."'");


//select db

$strSQL = "SELECT Company_Id, Company_Name, sess FROM tbl_company WHERE Email = '". $_POST['Email'] ."' AND Password = '". $_POST['Password'] ."' LIMIT 0, 1";


//connect
$dbQuery = mysql_query($strSQL,$MyDB) or die(mysql_error());

$dbResults = mysql_num_rows($dbQuery);

if ($dbResults > 0) {

	// user is authorized

	while($dbx = mysql_fetch_row($dbQuery)) {




		$_SESSION['SessID'] = session_id();


		//changed below from 0 to 16
		//$_SESSION['sess'] = $dbx['16'];

		//$session = $dbx['16'];

		//$session=$_SESSION['sess'];
		//$session=$_SESSION['SessID'];

		//added the insert


			//$_SESSION['SessID'] = session_id();

		$_SESSION['sess'] = $dbx['sess'];

		$session=$_SESSION['sess'];





	setcookie("Name", $dbx[1], time()+3600);

	}

}



// mysql_free_result($dbQuery);

include("admin/includes/db_close.php");

}



if ($dbResults > 0) {

// user login success

header("Location: MyAccount.php?sess=$session");

} else {

// user login failed

header("Location: index.php?error=1");

}



?>

 

The problem with this code is that it's giving me the link Myaccount.php?sess= without anything when I login. Also, I had another question, how can I make sure that users can't see other users info, that's the main problem.

 

 

[bruckerrlb]

If I need to explain anything better, please let me know

[roopurt18]

<?php
session_start();

if ($_POST['Email'] && $_POST['Password']) {
//db info
    include("admin/conf.php");
//open db
    include("admin/includes/db_open.php");

    $_POST['Email']=trim($_POST['Email']);

    $_POST['Password']=trim($_POST['Password']);
    

    // I DONT KNOW WHY YOU'D WANT TO DO THIS
    //This is how I want to show the company, instead of using their number, I'd rather have a random number stored in the db
    //$randval=mt_rand(); //TODO delete me
    
    // NOT NECESSARY
    //store it in the db
    //mysql_query("UPDATE tbl_company SET sess = '$randval' WHERE Email ='". $_POST['Email'] ."' and Password='". $_POST['Password'] ."'");

    
    //select db
    // TODO ESCAPE YOUR DATABASE VALUES WITH mysql_real_escape_string()
    $strSQL = "SELECT Company_Id, Company_Name FROM tbl_company WHERE Email = '". $_POST['Email'] ."' AND Password = '". $_POST['Password'] ."' LIMIT 0, 1";


    //connect
    // TODO GET RID OF OR DIE(), IT'S TERRIBLE (See the tutorial on the main PHP freaks page)
    $dbQuery = mysql_query($strSQL,$MyDB) or die(mysql_error());

    $dbResults = mysql_num_rows($dbQuery);

    if ($dbResults > 0) {

        // user is authorized

        while($dbx = mysql_fetch_row($dbQuery)) {




            $_SESSION['CompanyId'] = $dbx['Company_Id'];


            //changed below from 0 to 16
            //$_SESSION['sess'] = $dbx['16'];
            
            //$session = $dbx['16'];

            //$session=$_SESSION['sess'];
            //$session=$_SESSION['SessID'];
            
            //added the insert
            
            
            //$_SESSION['SessID'] = session_id();

            // TODO NOT SURE WHAT THESE ARE
            //$_SESSION['sess'] = $dbx['sess'];

            //$session=$_SESSION['sess'];
            
            
            
            
            // Don't see why you need this.
            //setcookie("Name", $dbx[1], time()+3600);

        }

    }

            

    // mysql_free_result($dbQuery);

    include("admin/includes/db_close.php");

}



if ($dbResults > 0) {

    // user login success

    header("Location: MyAccount.php");

} else {

    // user login failed

    header("Location: index.php?error=1");

}



?>

 

Then in MyAccount.php

<?php
session_start();
print_r( $_SESSION ); // You should see CompanyId, which you can use to identify
                        // and the user is unable to change it
?>

[bruckerrlb]

That's great, thanks for the comments also! I had one more question, it's the core of the program. I updated the program with the comments and the todo's but right now, if a user logs in, they can see all the records for all users. I didn't really understand what you ment by "You should see CompanyId, which you can use to identify". Before, the user went to his information by clicking on a link like:

index.php?Company_Id=25

But now that it's just index.php, all users info is showing up. How can I keep it as just having the specific users info showing up?

 

Thanks again

[greatstar00]

i think u should use a database to give them permissoins, like which they can go, if they can view some of all your users

if they can only view them self, then u dont need db

have a code to check the follow (based on condition)

if the user can view themselves only

just check the company id match the $_GET['company_id']

 

if the user can see a few others

select their permission from database, or from text file, store them in array

use in_array($company_id, $db_company_id)  //<<this isnt right syntax i think

[roopurt18]

But now that it's just index.php, all users info is showing up. How can I keep it as just having the specific users info showing up?

 

Your old index.php is probably like:

<?php
$company_id = $_GET['CompanyId'];
// now do stuff based off $company_id
?>

 

Your login form should now be saving CompanyId in the session, so your new pages can just use that.

<?php
session_start();
$CompanyID = $_SESSION['CompanyId'];
// now that company ID is set, it should work just the same as before

[bruckerrlb]

That was it, it's good to go now, thank you very much!