big, enourmous, huge, security problem.

[DeanWhitehouse]

I have a site, where the users can have profiles, i was just testing something when i found this security issue.

When i view the page listing the members and click on one, i become that member, i can edit there settings everything. How can i fix this. this is my member page code.

<?php

require_once 'db_connect.php';
require_once 'nav_bar.php';
require_once 'logged_in.php';

if ($_SESSION['is_valid'] == true){
if (isset($_GET['id'])) {
if ((int) $_GET['id'] > 0) {
$user_id = $_GET['id'];
$sql = "SELECT * FROM $user WHERE `user_id`='{$user_id}' LIMIT 0,1;";
$result = mysql_query($sql);
$row = mysql_fetch_assoc($result);
$username = $row['user_name'];
$email = $row['user_email'];
echo "$username<br>";
$show_email = $row['show_email'];
if ($show_email == 1)
{ 
echo "Email:<a href='mailto:$email'>$email</a>";

}
elseif ($show_email == 0)
{
echo "Email:Hidden";
}
exit();
}
else {
echo "Invalid user ID passed to page! <br />";
echo "<a href=\"members.php\">Return to user list</a>";
exit();
}
}
//No ID passed to page, display user list:
$query = "SELECT user_id, user_name FROM $user";
$result = mysql_query($query) or die("Error:" . mysql_error());
if (mysql_num_rows($result) > 0) {
echo "User List:<br />";
while ($row = mysql_fetch_assoc($result)) {
  echo '<a href="?id=' . $row['user_id'] . '">' . $row['user_name'] . '</a><br />';
}
}
}
else
{
echo "Please login to view this page.";
}
?>

 

this is a big risk, please help

[p2grace]

Where are you saving session information?  I'm assuming the system knows whose logged in by a session var correct?

[Northern Flame]

can you also show logged_in.php

[DeanWhitehouse]

this is logged_in.php

this also has the login form in it

<?php
if (isset($_GET['logout']))
{
setcookie("cookname", $_SESSION['username'], time() - 3600, "/");
setcookie("cookpass", $_SESSION['user_password'], time() - 3600, "/");
session_unset();
session_destroy();
}
if ($_SESSION['is_valid'] == true)
{
if ($_SESSION['user_level'] == 2)
{
	?>
	<table class='logged_in'><tr><td>
	<p>Welcome, <br><?php echo $_SESSION['username']; ?>
	<br><a href='user_profile.php?id=<?php echo $_SESSION['user_id']; ?>'>User Profile</a><br>
	<a href='user_setting.php'>Settings</a><br>
	<a href="<?php print $_SERVER["PHP_SELF"]; ?>?logout=true">Logout</a><br />
	</td></tr><tr><td>Logged In</td></tr></table></p>
	<?php 
}

if ($_SESSION['user_level'] == 1)
{
	?>
	<table class='logged_in'><tr><td>
	<p>Welcome, <?php echo $_SESSION['username']; ?>
	<br><a href='user_profile.php?id=<?php echo $_SESSION['user_id']; ?>'>User Profile</a><br>	
	<a href='user_setting.php'>Settings</a><br>
	<a href='admin_centre.php'>Admin Area</a><br>
	<a href="<?php print $_SERVER["PHP_SELF"]; ?>?logout=true">Logout</a><br />
	</td></tr><tr><td>Logged In</td></tr></table></p>
	<?php
}
}

else
{
require_once 'db_connect.php';

if ($_SESSION['is_valid'] == false)
{
	if (isset($_POST['login']))
	{

	$user_name = $_POST["user_name"];        
	$user_password = $_POST["user_password"]; 
	$cookiename = forumcookie;   
	$verify_username = strlen($user_name);
	$verify_pass = strlen($user_password);
		if ($verify_pass > 0 && $verify_username > 0)
		{
		$salt = substr($user_password, 0, 2);
		$userPswd = crypt($user_password, $salt);
		$sql = "SELECT * FROM `$user` WHERE user_name='$user_name' AND user_password='$userPswd' LIMIT 1;";
		$result = mysql_query($sql);
			if (mysql_num_rows($result) == 1)
			{
			$row = mysql_fetch_assoc($result);
			$user_level = $row['userlevel'];
				if ($user_level == 1) 
				{
				$login_check = @mysql_fetch_array(mysql_query("SELECT * from `$user` WHERE user_name = '$_GET[u]' AND user_password = 	'$_GET[p]'"));

				$userright = array($login_check['user_name'], $login_check['userlevel']);
				$s_userpass = serialize($userpass);
				$_SESSION['username'] = $row['user_name'];
				$_SESSION['user_password'] = $row['user_password'];
				$_SESSION['user_level'] = $row['userlevel'];
				$_SESSION['user_id'] = $row['user_id'];
				header("Location:http://".$_SERVER[HTTP_HOST].$_SERVER[REQUEST_URI]);
				$_SESSION['is_valid'] = true;
					 if(isset($_POST['remember']))
					 {
    					  setcookie("cookname", $_SESSION['username'], time()+60*60*24*100, "/");
    					  setcookie("cookpass", $_SESSION['user_password'], time()+60*60*24*100, "/");
   			}
			} 
		elseif ($user_level == 2){    
			$login_check = @mysql_fetch_array(mysql_query("SELECT * from `$user` WHERE user_name = '$_GET[u]' AND user_password = '$_GET[p]'"));

			$userright = array($login_check['user_name'], $login_check['userlevel']);
		$s_userpass = serialize($userpass);
	$_SESSION['username'] = $row['user_name'];
	$_SESSION['user_password'] = $row['user_password'];
	$_SESSION['user_level'] = $row['userlevel'];
	$_SESSION['user_id'] = $row['user_id'];
	header("Location:http://".$_SERVER[HTTP_HOST].$_SERVER[REQUEST_URI]);
	$_SESSION['is_valid'] = true; //change the session variable name to what you want, just remember it for all files
	 if(isset($_POST['remember'])){
    	  setcookie("cookname", $_SESSION['username'], time()+60*60*24*100, "/");
    	  setcookie("cookpass", $_SESSION['user_password'], time()+60*60*24*100, "/");
   		}
	}
}
else{
	echo "Login failed. Username and Password did not match database entries.";    
}
}

else
{
echo "Form was not completed. Please go back and make sure that the form was fully completed.";    
}
}
$server = str_replace("?logout=true","",$_SERVER['PHP_SELF']);
?> 

<html>
<table bgcolor='#999999' align='right'><form action="<?php echo $server ?>" method='POST'>
<tr><td>Username: </td><td><input type='text' name='user_name' /><br /></td></tr>
<tr><td>Password:</td><td> <input type='password' name='user_password' /><br /></td></tr>
<tr><td><input type="hidden" name="login" value="true"><input type="submit" value="Submit"></td></tr>
<tr><td><input type="checkbox" value="1" name="remember"> Remember Me </td></tr><tr><td><a href="register.php">[Register]</a></td></tr><tr><td><a href="forgot_password.php">[Forgot Password?]</a></td></tr></table>
</form>
</html>

<?php 
mysql_close();
}
else
{
header("Location:http://".$_SERVER[HTTP_HOST]);
}
}
?>

 

and yes they are identified by there user_id stored in a variable

[DeanWhitehouse]

i can't see how this is happening, as it isn;t setting any thing on the members page

[p2grace]

It's not hard, it's just a lot of code to take time to sit down and go through (I'm currently at work).  Try saving the information from the search as different variable names, if your post data is the same as your login post data perhaps it's getting overwritten.  Use different post vars for the search.

[DeanWhitehouse]

hmm ,tried that didn't work. I just can see how it's doing this.

Should i identify people not just by user_id but user_name?

[p2grace]

Start echoing the session variables.  If they change after clicking on a user then the data is being overwritten

[DeanWhitehouse]

they are echoed,in the logged_in script, it echos there username, and it does change, but i don't know how

[DarkWater]

Check if the session is overwritten somehow in:

 

require_once 'db_connect.php';

require_once 'nav_bar.php';

require_once 'logged_in.php';

 

Any of those.

[DeanWhitehouse]

logged_in is already posted here, and heres nav_bar and db_connect, i don;t think it is.

 

<?php

// Random Game Design: PHP Website Template
//  Version 1
//  Copyright Dean Whitehouse, 2008

//  Include config file
require_once 'config.inc.php';
require_once 'config_table.inc.php';

// Connect to database
mysql_connect($dbhost,$dbuser,$dbpass)
or die('Could not connect: ' . mysql_error());

// Select database
mysql_select_db($dbname)
or die('Could not find the database: ' . mysql_error());

ob_start();
session_start();


?>

db_connect.php

 

<?php 
$server = "http://".$_SERVER[HTTP_HOST]
;
if ($_SESSION['is_valid'] == true){
if ($_SESSION['user_level'] == 2){
echo "<table class='nav_bar'><tr><td>
<a class='nav_bar' href='$server'>Home</a>
</td></tr><tr><td><font color='white'>Logged In</font></td></tr>
<tr><tr><a href='members.php'>Members</a></td></tr></table>";
}
if ($_SESSION['user_level'] == 1){
echo "<table class='nav_bar'><tr><td>
<a class='nav_bar' href='$server'>Home</a>
</td></tr><tr><tr><a href='members.php'>Members</a></td></tr></table>";
}
}
else
{
echo "
<table class='nav_bar'><tr><td>
<a class='nav_bar' href='$server'>Home</a>
</td></tr><tr><tr><a href='members.php'>Members</a></td></tr></table>";
}
?>

nav_bar.php

[DeanWhitehouse]

also ,i just found out that the user level is also transfered so the user, can now see the admin area

[PFMaBiSmAd]

Use a phpinfo(); statement and tell us what the register globals setting is.

 

Without looking through all your code and figuring out how each bit is related, the symptom seems like it is the security hole created by register globals where session variables can be set by post/get/cookie/program variables with the same name as the session variable.

 

If it turns out that register globals are on, then turn them off (using a .htaccess file or a local php.ini file) and see if the problem goes a way.

[DarkWater]

You know, I was on the main page just a second ago and this was the "recent post"...I just saw:

"Re: big, enourmous, huge..."  (that's all)

 

And I was like, whoa, how misleading is THAT thread title, and then I remembered it was this one.  D:  Might want to rethink your thread title next time.

[haku]

This is a big problem here:

 

if (isset($_GET['id'])) {
if ((int) $_GET['id'] > 0) {
$user_id = $_GET['id'];

 

Since the user's ID is set as a variable in the URL, then anyone can come along and put in a different user's ID into the URL and become that user. You need to change this so that it is saved in a SESSION variable.

 

Sorry if this has already been mentioned in the thread, I only did a quick scan as I am also at work and don't have a lot of time to go through it line by line.

[DeanWhitehouse]

the user_id is also saved in a session, but this session seems to be overwrote when i click on another users id

[PFMaBiSmAd]

Like I posted above, find and tell us what the register globals setting is. If it is on, turn it off and see if that corrects the problem. If people give you suggestions to try and you don't bother, it will end up taking a really long time for you to solve your problem.

 

The symptom of session variables being overwritten by post/get/cookie/program variables is exactly why register globals were turned off by default a long time ago and have been eliminated in upcoming php6.

[DeanWhitehouse]

sorry, i was just saying, i haven't had access to the files yet, i will try when i have time , i was just saying

[PFMaBiSmAd]

Your post #15 only restated the symptoms and did not provide any additional information.

 

So far, you have not shown your "edit" code. You stated that you become a member once you click on their link (the code in the first post). The code in the first post only displays the user information. How do you get to the "edit" code from that page? Also, it would take seeing the "edit" code to be able to see what it is using to determine which user to edit.

[PFMaBiSmAd]

I tested your code, bypassing items I don't have... and I was able to get the ?id= parameter on the "User Profile" link, which is from $_SESSION['user_id'] to change to the members.php?id= link value, when register globals are on. With register globals off, the $_SESSION['user_id'] stayed as the logged in user_id.

 

Here is what it is doing -

 

When you assign a value to $user_id in the following line of code -

 

$user_id = $_GET['id'];

 

because register globals are on, this sets $_SESSION['user_id'] to that same value, which changes the logged in user id to that user_id.

 

Your title - "big, enormous, huge, security problem." is the definition of register globals.

 

Here is my normal rant about register globals -

 

Register globals were turned off by default in php 4.2 in the year 2002 (I think it was in April, so that was six full years ago) because of this session security hole. No new books, tutorials, code, hosting accounts... should have been created after that point in time that had register globals turned on or relied on register globals being on. Thank goodness register globals are finally, a few years too late, being removed in php6.

[DeanWhitehouse]

To turn it off(fix it) i need to create a .htacces file or a php.ini file?? what do i need to write in these

[DeanWhitehouse]

how can i fix this?

[PFMaBiSmAd]

The method of turning off register globals is dependent on how php is installed on your web server and to some extent the type of web server. You should ask your hosting company (or they probably have a FAQ) to find out the exact method.

 

Since the problem is caused by post/get/cookie/session/program variables with the same name, don't you suppose that one way to solve the problem would be to change some variable names so they are all unique?