View Issue Details

IDProjectCategoryView StatusLast Update
0009664mantisbtsecuritypublic2008-10-23 10:29
Reporterseiji Assigned Tojreese  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
PlatformOSGentoo LinuxOS Version
Product Version1.1.2 
Target Version1.1.3Fixed in Version1.1.3 
Summary0009664: Logout without unsetting session cookie
Description

MantisSession#destory() uses session_destroy(). But it does not unset session cookie.
Logout should unset session cookie for security reason.
see attached patch.

Steps To Reproduce
  1. check session id.( with Firefox addon LiveHttpHeader)
  2. logout
  3. login
  4. check session id again.
Additional Information

1.1.2 and 1.2.0

Tagspatch

Relationships

related to 0009665 closedjreese Logout without unsetting session cookie 

Activities

2008-09-26 11:13

 

sessionid.patch.txt (1,178 bytes)
Index: core/authentication_api.php
===================================================================
--- core/authentication_api.php	(revision 211)
+++ core/authentication_api.php	(working copy)
@@ -265,6 +265,8 @@
 			auth_http_set_logout_pending( true );
 		}
 
+		session_clean();
+
 		return true;
 	}
 
Index: core/session_api.php
===================================================================
--- core/session_api.php	(revision 211)
+++ core/session_api.php	(working copy)
@@ -58,6 +58,8 @@
 		session_cache_limiter( 'private_no_expire' );
 		if ( isset( $_SERVER['HTTPS'] ) && ( strtolower( $_SERVER['HTTPS'] ) != 'off' ) ) {
 			session_set_cookie_params( 0, config_get( 'cookie_path' ), config_get( 'cookie_domain' ), true, true );
+		} else {
+			session_set_cookie_params( 0, config_get( 'cookie_path' ), config_get( 'cookie_domain' ), false, true );
 		}
 		session_start();
 		$this->id = session_id();
@@ -85,7 +87,10 @@
 	}
 
 	function destroy() {
-		unset( $_SESSION );
+		if ( isset( $_COOKIE[ session_name() ] ) && !headers_sent() ) {
+			gpc_set_cookie( session_name(), '', time() - 42000 );
+		}
+		//unset( $_SESSION );
 		session_destroy();
 	}
 }
sessionid.patch.txt (1,178 bytes)
jreese

jreese

2008-09-26 20:42

reporter   ~0019465

Why does it matter if the cookie is still on the client after session_destroy() has been called? At that point, the cookie links to nothing, so I don't understand what the security risk could be...

seiji

seiji

2008-09-26 21:27

reporter   ~0019466

See PHP Manual
http://www.php.net/manual/en/function.session-destroy.php

In order to kill the session altogether, like to log the user out, the session id must also be unset. If a cookie is used to propagate the session id (default behavior), then the session cookie must be deleted. setcookie() may be used for that.

jreese

jreese

2008-09-26 21:57

reporter   ~0019467

I can see that deleting the cookie has a purpose, but I'm still not understanding what the security issue is with leaving the cookie on the client system. The session data is destroyed on the server, and a new session would need to be created at the next page load, so what's the security risk of leaving cookies client-side, especially considering that any client could simply ignore the cookie deletion request anyways...

seiji

seiji

2008-09-27 00:08

reporter   ~0019468

Certainly, the session data is destroyed.
(But MantisPHPSession#destroy is never called from anywhere... bug? )

However, The old session id is used when logging in mantis again.

  1. User A logs in mantis.
  2. User B steals A's session id by XSS.
  3. A logs out.
    a) The session data is destroyed. the session cookies are not.
  4. A logs in again without closing browser.
    a) The new session is created. but session cookies(3-a) are used.
  5. B suceeds in the session hijack with session cookies(3-a).

OWASP Session Management
http://www.owasp.org/index.php/Session_Management#Session_Tokens_on_Logout

thanks.

jreese

jreese

2008-09-27 09:57

reporter   ~0019469

OK, that's the explanation I was looking for; thank you. And on your middle remark, it probably is a bug if it doesn't get called anywhere. I will work on getting this put in ASAP.

jreese

jreese

2008-09-27 10:33

reporter   ~0019470

Fix has been committed to SVN 1.1.x, r5594.

giallu

giallu

2008-10-23 10:29

reporter   ~0019656

This is now CVE-2008-4689

Related Changesets

MantisBT: master cbd8c1e5

2008-09-27 14:23:58

jreese

Details Diff
Fix 0009664: PHP session cookies were not destroyed, and session_clean() was never called.

git-svn-id: http://mantisbt.svn.sourceforge.net/svnroot/mantisbt/trunk@5593 <a class="text" href="/?p=mantisbt.git;a=object;h=f5dc347c">f5dc347c</a>-c33d-0410-90a0-b07cc1902cb9
Affected Issues
0009664
mod - core/authentication_api.php Diff File
mod - core/session_api.php Diff File

MantisBT: master-1.1.x 9ddf7eef

2008-09-27 14:28:01

jreese

Details Diff
Fix 0009664: PHP session cookies were not destroyed, and session_clean() was never called.

git-svn-id: http://mantisbt.svn.sourceforge.net/svnroot/mantisbt/branches/BRANCH_1_1_0@5594 <a class="text" href="/?p=mantisbt.git;a=object;h=f5dc347c">f5dc347c</a>-c33d-0410-90a0-b07cc1902cb9
Affected Issues
0009664
mod - core/session_api.php Diff File
mod - core/authentication_api.php Diff File

Issue History

Date Modified Username Field Change
2008-09-26 11:13 seiji New Issue
2008-09-26 11:13 seiji File Added: sessionid.patch.txt
2008-09-26 11:15 seiji Tag Attached: patch
2008-09-26 20:42 jreese Note Added: 0019465
2008-09-26 20:42 jreese Assigned To => jreese
2008-09-26 20:42 jreese Status new => feedback
2008-09-26 21:27 seiji Note Added: 0019466
2008-09-26 21:57 jreese Note Added: 0019467
2008-09-27 00:08 seiji Note Added: 0019468
2008-09-27 09:57 jreese Note Added: 0019469
2008-09-27 09:57 jreese Status feedback => assigned
2008-09-27 09:57 jreese Target Version => 1.1.3
2008-09-27 09:58 jreese Issue cloned: 0009665
2008-09-27 09:58 jreese Relationship added related to 0009665
2008-09-27 10:33 jreese Note Added: 0019470
2008-09-27 10:33 jreese Status assigned => resolved
2008-09-27 10:33 jreese Fixed in Version => 1.1.3
2008-09-27 10:33 jreese Resolution open => fixed
2008-10-18 18:32 giallu Status resolved => closed
2008-10-20 16:46 Changeset attached master 2bda355c =>
2008-10-20 20:19 Changeset attached master-1.1.x 05e4cd9a =>
2008-10-23 10:29 giallu Note Added: 0019656
2008-10-23 10:29 giallu Category authentication => security
2008-11-11 08:32 jreese Changeset attached master cbd8c1e5 =>
2008-11-11 08:45 jreese Changeset attached master cbd8c1e5 =>
2008-11-11 09:03 jreese Changeset attached master-1.1.x 9ddf7eef =>