bruckerrlb Posted January 20, 2010 Share Posted January 20, 2010 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. Quote Link to comment Share on other sites More sharing options...
bruckerrlb Posted January 20, 2010 Author Share Posted January 20, 2010 If I need to explain anything better, please let me know Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted January 20, 2010 Share Posted January 20, 2010 <?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 ?> Quote Link to comment Share on other sites More sharing options...
bruckerrlb Posted January 20, 2010 Author Share Posted January 20, 2010 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 Quote Link to comment Share on other sites More sharing options...
greatstar00 Posted January 20, 2010 Share Posted January 20, 2010 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 Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted January 20, 2010 Share Posted January 20, 2010 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 Quote Link to comment Share on other sites More sharing options...
bruckerrlb Posted January 22, 2010 Author Share Posted January 22, 2010 That was it, it's good to go now, thank you very much! Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.