My Users keep getting logged in as others.

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

My Users keep getting logged in as others.

Post by s.dot »

Post Edited: My login code was orginally posted here, but the problem didn't lie in the code. Therefore I removed it. =]
Last edited by s.dot on Thu Aug 17, 2006 5:01 pm, edited 3 times in total.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

Oh, how I check if they're logged in is...

Code: Select all

if(isset($_SESSION['id'])){
   //they're logged in,
   //i now use $theperson to tell who they are
}
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

Nobody, eh?
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

Well, if I had this problem, I'd ask: "Hmm... this means that the user_id is getting changed somewhere along the way. It seems that you're overloading the passed username with an array of the returned db query. Some consistency checks there would be a good idea.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

Last edited by s.dot on Thu Aug 17, 2006 5:02 pm, edited 1 time in total.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

From what I see, there are two places it could be failing.

It could be failing here, but that is unlikely.

Code: Select all

"SELECT `id`,`username`,`activated`,`timeoffset`,`password`,`password_is_sha`,`passwordsha`,`time_activated` FROM `users` WHERE `username` = '$username'") or die(mysql_error());
I think the problem is here.

Code: Select all

$session_result = mysql_query("SELECT `session` FROM `users` WHERE `id` = '$theperson' LIMIT 1") or die(mysql_error());
I believe $theperson is not a unique ID, and it's pulling session data for the first matching record, regardless of whether it's the correct record or not.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

Hmm.. this seems to still be happening. I haven't experienced it myself but I've heard about it from others. So, the next link would be my logout page. My users are redirected to this if the session credentials don't match what's stored in the database.

Does this effectively clear everything from the session (effectively logging them out)?

Code: Select all

<?php

session_start();
if(isset($_COOKIE[session_name()])){
	setcookie(session_name(), '', time()-42000, '/');
}
if(isset($_SESSION['id'])){
	session_destroy();
}
$_SESSION = array();
header("Location: index.php");

?>
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

a while back myself and a few colleagues discovered we were able to browse the same site whilst at work, but occasionally as each other. After some digging, we found that the session ID was being passed as a get var, and possibly the cookie info is being regenerated on every page request.

the conclusion at the end came from both of us using the same proxy and the proxy was caching the pages (with links with the session ID) and if we didn't notice the username was different, we would steal each others session upon clicking any of the otherwise harmless links.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

It seems a randomly generated cookie value stored in the database and checked on each pageload would kill the possibility of that problem.. as the cookie would be only unique to that person's machine. But using sessions AND cookies... hmm.. is that a common practice?
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Problem is, if the proxy is pulling from it's cache, you won't even get a request to the original page, most often. Although Jenk's descriptions sounds like a possibly poorly implemented proxy.

Ignoring the local path redirection, the call to setcookie() may not write the headers when used with the redirect. In some installs I've noticed a short-circuiting effect when using a redirect and setting cookies (and sessions) at the same time.

When performing a logout, I would use a page level redirection. session_write_close() may be a good idea to use here as well.
choppsta
Forum Contributor
Posts: 114
Joined: Thu Jul 03, 2003 11:11 am

Post by choppsta »

I have come across something similar to what Jenk and Feyd describe with a client who would often see pages from different sessions across different machines in the same office because of their proxy server serving up cached pages.

I got round it by outputting the following headers:

Code: Select all

header("Cache-Control: private");
header("Pragma: no-cache");
This should stop proxies from caching your pages.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

scottayy wrote:It seems a randomly generated cookie value stored in the database and checked on each pageload would kill the possibility of that problem.. as the cookie would be only unique to that person's machine. But using sessions AND cookies... hmm.. is that a common practice?
cookies storing the session ID, they are not entirely different entities.

the problem with the proxy was it was caching to save the company bandwidth where possible. (we assume)

the sites main index, we both would access just by entering the url, as you do: http://www.example.com

The proxy looks in it's cache to see if within the last 'x' minutes a page with the same URI has been requested, if it has, serve the cached page. It may check through access IP etc. as well, but we are in a very large corporation thus through routing etc. we may well still appear as the same machine to the proxy.

That cached page could hold a link:

Code: Select all

<a href="http://www.example.com/page2.php?PHPSESSID=112ujbin4523mn123">Link</a>
If the code behind the website resets the users cookie to contain the latest session ID, which may be obtained from the querystring as above, then I would take that session. The problem being back on the proxy - is that my session, or my colleages?

In other words, it was a big bag of smaller problems that lead to a bigger one :)
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

Wow. I wrote a simple test script to display information about one of the users this was happening to. I wanted her to run it now to show normal output, then compare that output against the output when she is logged in as another user.

I didn't even have to wait till the comparison for a surprise!

This is the output:

Code: Select all

Session Info

Array
(
[id] => 10307
[username] => _tanya_
[timeoffset] => -3
[smiley_pref] => 6
)

Cookie Info

Array
(
[usern] => deleted
[rememberme] => deleted
[PHPSESSID] => deleted
[fontprefs] => a:6:{s:4:\"bold\";s:1:\"1\";s:9:\"underline\";s:1:\"0\";s:7:\"italics\";s:1:\"1\";s:4:\"face\";s:7:\"Verdana\";s:4:\"size\";s:1:\"2\";s:5:\"color\";s:7:\"#5F9EA0\";}
[doSearch] => a:11:{s:6:\"action\";s:8:\"doSearch\";s:6:\"gender\";s:1:\"1\";s:7:\"age_min\";s:2:\"18\";s:7:\"age_max\";s:2:\"35\";s:8:\"distance\";s:2:\"15\";s:4:\"unit\";s:1:\"1\";s:3:\"zip\";s:0:\"\";s:6:\"online\";s:1:\"3\";s:9:\"scriteria\";s:1:\"1\";s:12:\"scriteriaval\";s:7:\"_Tanya_\";s:4:\"sort\";s:1:\"1\";}
)

Misc. Info

ID: 10307
Username: _tanya_
Current Session: deleted
Database Session: deleted
Browser: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)
IP: x.x.x.x (removed for privacy purposes)
Here is the script:

Code: Select all

<?php
session_start();
require 'includes/db_connect.php';
require 'includes/session_check.php';

if(isset($_SESSION['id']))
{
	echo '<pre>';
	print_r($_SESSION);
	echo '</pre>';

	echo '<br /><br />';
	echo '<pre>';
	print_r($_COOKIE);
	echo '</pre>';
	
	echo '<b>ID:</b> '.$theperson;
	echo '<br />';
	echo '<b>Username:</b> '.$theperson_username;
	echo '<br />';
	echo '<b>Current Session:</b> '.session_id();
	echo '<br />';
	echo '<b>Database Session:</b> ';
	$r = mysql_query("SELECT `session` FROM `users` WHERE `id` = '$theperson' LIMIT 1") or die(mysql_error());
	if($a = mysql_fetch_assoc($r))
	{
		echo $a['session'];
	} else
	{
		echo 'Could not be located.';
	}
	echo '<br />';
	echo '<b>Came From:</b> '.$_SERVER['HTTP_REFERER'];
	echo '<br />';
	echo '<b>Browser:</b> '.$_SERVER['HTTP_USER_AGENT'];
	echo '<br />';
	echo '<b>IP:</b> '.$_SERVER['REMOTE_ADDR'];
} else
{
	echo 'You have reached this page in error.';
}
?>
Notice all the "deleted" values? How can a session have a session_id() of 'deleted'? Same with cookie values? Nothing in my scripts sets these values to deleted.

I am pretty much shocked by the output.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Code: Select all

SELECT COUNT(*) FROM `USERS` WHERE `id` = '<_tanya_s id>'
How many rows do you get from that?
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

1 row returned. The id field is an auto_incremeted primary key.

However, when I..

Code: Select all

SELECT count(*) FROM `users` WHERE `session` = 'deleted'
It returns 17 rows.

You can see in my login script above that when they login, I store the session_id() in the database. Never in any of my scripts do I update the session data or cookie data to a "deleted" value. Which is why I find this quite odd.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
Post Reply