unidox Posted September 5, 2007 Share Posted September 5, 2007 I have a cms, and I just found out that its not secure. I was just wondering how I can make it more secure. Right now, it sets cookies of the access level and the user level, when someone logs in. And in each page, to restrict access levels, it checks the cookie access to determine its access level. Thanks in advance! So here are my files: Code to check the user level on each page: <?php if ($_COOKIE['uniqueid']) { ?> <?php $a = $_COOKIE['access']; global $levels; if ($a <= $levels[pages]) { PAGE CONTENT } <?php if ($a > $levels[pages]) { if (!$_REQUEST['m']) { require_once("inc/db.inc.php"); require_once ("inc/func2.inc.php"); getHeader(); echo "Sorry, you dont have access to this page!"; } } login.php: <?php $page = "login"; require_once ("inc/db.inc.php"); require_once ("files/login.php"); if ($_REQUEST['m']) { if ($_REQUEST['m'] == "1") { $loginpass = $_POST['login_pass']; $password = md5($loginpass); $loginname = $_POST['login_name']; $checkrows = mysql_query ("SELECT * FROM cp_users WHERE username='$loginname' && password='$password'") or die (mysql_error()); $rowcount = mysql_num_rows ($checkrows); if ($rowcount == "0") { showError("User/Login Error"); } if ($rowcount != "0") { header ("Location: index.php?page=admin"); $time = date("h:i:a"); $date = date("m/d/Y"); $last_logged = $time . "\n(" . $date . ")"; $ip = getenv ("REMOTE_ADDR"); MYSQL_QUERY("UPDATE cp_users SET last_logged='$last_logged', cur_ip='$ip' WHERE username='$loginname'") or die (mysql_error()); while ($mysql=mysql_fetch_array($checkrows)) { setcookie("access", $mysql[access],time()+60*60*24*30); } setcookie ("uniqueid",$loginname,time()+60*60*24*30); exit; } } elseif ($_REQUEST['m'] == "2") { header ("Location: index.php?page=login"); setcookie ("uniqueid"); setcookie ("access"); exit; } } else { if ($_COOKIE['uniqueid'] == "") { $checkfields = "login_name&login_pass"; $errors = "Enter a username&Enter a password!"; $titles = "Username:&Password:"; $fields = "login_name&login_pass"; $type = "text&password"; $size = "30&30"; $maxlength = "25&25"; createJSValid($checkfields,$errors); createForm($titles,$fields,$type,$size,$maxlength,'1','','','','1'); } else { showError("You are already logged in, <a href=\"" . $_SERVER['PHP_SELF'] . "?page=login&m=2\">logout?</a><br /><br /><a href='index.php?page=admin'>Admin Home</a>"); } } ?> Quote Link to comment Share on other sites More sharing options...
Jessica Posted September 5, 2007 Share Posted September 5, 2007 Use sessions, not cookies. Cookies can be edited. Quote Link to comment Share on other sites More sharing options...
unidox Posted September 5, 2007 Author Share Posted September 5, 2007 How would I do that? Quote Link to comment Share on other sites More sharing options...
Jessica Posted September 5, 2007 Share Posted September 5, 2007 Think about it for a minute before asking. Then google. Quote Link to comment Share on other sites More sharing options...
unidox Posted September 5, 2007 Author Share Posted September 5, 2007 well, i tried this, but now I get an error: Parse error: syntax error, unexpected '}' in /home/clansuni/public_html/adm_files/login.php on line 26 Here is the updated code: <?php session_start(); $page = "login"; require_once ("inc/db.inc.php"); require_once ("files/login.php"); if ($_REQUEST['m']) { if ($_REQUEST['m'] == "1") { $loginpass = $_POST['login_pass']; $password = md5($loginpass); $loginname = $_POST['login_name']; $checkrows = mysql_query ("SELECT * FROM cp_users WHERE username='$loginname' && password='$password'") or die (mysql_error()); $rowcount = mysql_num_rows ($checkrows); if ($rowcount == "0") { showError("User/Login Error"); } if ($rowcount != "0") { header ("Location: index.php?page=admin"); $time = date("h:i:a"); $date = date("m/d/Y"); $last_logged = $time . "\n(" . $date . ")"; $ip = getenv ("REMOTE_ADDR"); MYSQL_QUERY("UPDATE cp_users SET last_logged='$last_logged', cur_ip='$ip' WHERE username='$loginname'") or die (mysql_error()); while ($mysql=mysql_fetch_array($checkrows)) { $_SESSION['access'] = $mysql[access] } $_SESSION['uniqueid'] = $loginname $_SESSION['password'] = $password exit; } } elseif ($_REQUEST['m'] == "2") { header ("Location: index.php?page=login"); session_destroy(); exit; } } else { if ($_COOKIE['uniqueid'] == "") { $checkfields = "login_name&login_pass"; $errors = "Enter a username&Enter a password!"; $titles = "Username:&Password:"; $fields = "login_name&login_pass"; $type = "text&password"; $size = "30&30"; $maxlength = "25&25"; createJSValid($checkfields,$errors); createForm($titles,$fields,$type,$size,$maxlength,'1','','','','1'); } else { showError("You are already logged in, <a href=\"" . $_SERVER['PHP_SELF'] . "?page=login&m=2\">logout?</a><br /><br /><a href='index.php?page=admin'>Admin Home</a>"); } } ?> Quote Link to comment Share on other sites More sharing options...
Jessica Posted September 5, 2007 Share Posted September 5, 2007 You're missing a ; at the end of a line. Also, $mysql[access] is poor code, write $mysql['access']; Quote Link to comment Share on other sites More sharing options...
unidox Posted September 5, 2007 Author Share Posted September 5, 2007 i fixed the error, but now it just keeps redirecting to the login page. Here is part of the func: $islogged = preg_match("/index.php?page=login/", $_SERVER['PHP_SELF']); if ($islogged == "0") { if ($_SESSION['uniqueid'] == "") { header ("Location: index.php?page=login"); exit; } } if ((!$_REQUEST['method']) || (!$_SESSION['uniqueid'])) { $access = $_SESSION['access']; if (array_search($page,$levels)) { if ($access <= $levels[$page]) { echo $access . $levels[$page]; showError('You do not have access to this page.'); exit; } } } Quote Link to comment Share on other sites More sharing options...
unidox Posted September 5, 2007 Author Share Posted September 5, 2007 bump Quote Link to comment Share on other sites More sharing options...
AdRock Posted September 5, 2007 Share Posted September 5, 2007 Have a look at this article about a login system using sessions http://www.olate.co.uk/articles/185 In the function that logs a user in, you could have a field in the database with their user level and encrypt that in the session and when you use the auth function you can add some extra code to compare the encrypted user level with the original user level in the session to see if it has been change and if it has don't allow the script to run. Quote Link to comment Share on other sites More sharing options...
trq Posted September 5, 2007 Share Posted September 5, 2007 Can you explain exactly what your trying to do here? $islogged = preg_match("/index.php?page=login/", $_SERVER['PHP_SELF']); if ($islogged == "0") { if ($_SESSION['uniqueid'] == "") { header ("Location: index.php?page=login"); exit; } } That piece makes little sense. Firstly, why are you using preg_match at all? Just check if $_GET['page'] == 'login'. Secondly, preg_match returns an int, your checking for a string. Really, its pretty hard to see what your trying to achive with this piece, you'll need to explain it. Quote Link to comment Share on other sites More sharing options...
unidox Posted September 6, 2007 Author Share Posted September 6, 2007 before, I was using cookies, and it used to be an integer, but they said Sessions is better, so I addedd sessions. Quote Link to comment Share on other sites More sharing options...
sneamia Posted September 6, 2007 Share Posted September 6, 2007 First, try isset or empty instead == ''. Second, I see no reason in using preg_match, as thorpe mentioned. There is no reason to include the overhead of the regex engine. Just do if ($_SERVER['PHP_SELF'] == '/index.php?page=login/') { if (empty($_SESSION['uniqueid'])) { header("Location: index.php?page=login"); exit; } } Right? Quote Link to comment Share on other sites More sharing options...
trq Posted September 6, 2007 Share Posted September 6, 2007 Even better.... <?php if ($_GET['page'] == 'login') { if (empty($_SESSION['uniqueid'])) { header("Location: index.php?page=login"); exit; } } ?> Quote Link to comment Share on other sites More sharing options...
sneamia Posted September 6, 2007 Share Posted September 6, 2007 Haha, didn't even see that. Quote Link to comment Share on other sites More sharing options...
unidox Posted September 7, 2007 Author Share Posted September 7, 2007 I tried it, but it keeps redirecting. grrr. what am I doing wrong? Quote Link to comment Share on other sites More sharing options...
trq Posted September 7, 2007 Share Posted September 7, 2007 Can we see some code where you set these session vars? ie: The relevant parts of your login script. Quote Link to comment Share on other sites More sharing options...
sneamia Posted September 7, 2007 Share Posted September 7, 2007 And does it redirect you to login or admin? Quote Link to comment Share on other sites More sharing options...
unidox Posted September 7, 2007 Author Share Posted September 7, 2007 If you want the code, look at the 1st post, I reverted everything back, cause it wasnt working. It redirects to the login page Quote Link to comment Share on other sites More sharing options...
trq Posted September 7, 2007 Share Posted September 7, 2007 Nowhere in your first post do you set any sessions. If you want help, you need to post the relevant code. Quote Link to comment Share on other sites More sharing options...
sneamia Posted September 7, 2007 Share Posted September 7, 2007 Here is the updated code: <?php session_start(); $page = "login"; require_once ("inc/db.inc.php"); require_once ("files/login.php"); if ($_REQUEST['m']) { if ($_REQUEST['m'] == "1") { $loginpass = $_POST['login_pass']; $password = md5($loginpass); $loginname = $_POST['login_name']; $checkrows = mysql_query ("SELECT * FROM cp_users WHERE username='$loginname' && password='$password'") or die (mysql_error()); $rowcount = mysql_num_rows ($checkrows); if ($rowcount == "0") { showError("User/Login Error"); } if ($rowcount != "0") { header ("Location: index.php?page=admin"); $time = date("h:i:a"); $date = date("m/d/Y"); $last_logged = $time . "\n(" . $date . ")"; $ip = getenv ("REMOTE_ADDR"); MYSQL_QUERY("UPDATE cp_users SET last_logged='$last_logged', cur_ip='$ip' WHERE username='$loginname'") or die (mysql_error()); while ($mysql=mysql_fetch_array($checkrows)) { $_SESSION['access'] = $mysql[access] } $_SESSION['uniqueid'] = $loginname $_SESSION['password'] = $password exit; } } elseif ($_REQUEST['m'] == "2") { header ("Location: index.php?page=login"); session_destroy(); exit; } } else { if ($_COOKIE['uniqueid'] == "") { $checkfields = "login_name&login_pass"; $errors = "Enter a username&Enter a password!"; $titles = "Username:&Password:"; $fields = "login_name&login_pass"; $type = "text&password"; $size = "30&30"; $maxlength = "25&25"; createJSValid($checkfields,$errors); createForm($titles,$fields,$type,$size,$maxlength,'1','','','','1'); } else { showError("You are already logged in, <a href=\"" . $_SERVER['PHP_SELF'] . "?page=login&m=2\">logout?</a><br /><br /><a href='index.php?page=admin'>Admin Home</a>"); } } ?> Quote Link to comment Share on other sites More sharing options...
unidox Posted September 8, 2007 Author Share Posted September 8, 2007 bump 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.