Close a loophole in CookieSessionProvider.patch
No One
Actions

Authored By

	Anomie
	Jan 30 2016, 4:11 PM

Size

2 KB

Referenced Files

None

Subscribers

None

Close a loophole in CookieSessionProvider.patch
View Options

	From 7cce0f29d4653fe949b6ce5df0cc1a3ccd386778 Mon Sep 17 00:00:00 2001
	From: Brad Jorsch <bjorsch@wikimedia.org>
	Date: Sat, 30 Jan 2016 10:54:24 -0500
	Subject: [PATCH] Close a loophole in CookieSessionProvider

	There's a crazy-small chance that someone could have a logged-out
	session (e.g. by logging out or visiting a page that creates a session
	despite being logged out), then the session expires, then someone else
	logs in and gets the same session ID (which is about a 1 in a
	quindecillion chance), then the first person comes in and picks up the
	second person's session.

	To avoid that, if there's no UserID cookie set (or the cookie value is
	0) then indicate that the SessionInfo is for a logged-out user.

	No idea if this is actually what happened in T125283, but it's worth
	fixing anyway.

	Bug: T125283
	Change-Id: I44096c69aa7bd285e4e2472959e8d892200c5f2c
	---
	includes/session/CookieSessionProvider.php \| 26 +++++++++++++++-----------
	1 file changed, 15 insertions(+), 11 deletions(-)

	diff --git a/includes/session/CookieSessionProvider.php b/includes/session/CookieSessionProvider.php
	index 2d01d1d..f989cbc 100644
	--- a/includes/session/CookieSessionProvider.php
	+++ b/includes/session/CookieSessionProvider.php
	@@ -104,11 +104,14 @@ class CookieSessionProvider extends SessionProvider {

	public function provideSessionInfo( WebRequest $request ) {
	$info = array(
	- 'id' => $this->getCookie( $request, $this->params['sessionName'], '' )
	+ 'id' => $this->getCookie( $request, $this->params['sessionName'], '' ),
	+ 'provider' => $this,
	+ 'forceHTTPS' => $this->getCookie( $request, 'forceHTTPS', '', false )
	);
	if ( !SessionManager::validateSessionId( $info['id'] ) ) {
	unset( $info['id'] );
	}
	+ $info['persisted'] = isset( $info['id'] );

	list( $userId, $userName, $token ) = $this->getUserInfoFromCookies( $request );
	if ( $userId !== null ) {
	@@ -128,21 +131,22 @@ class CookieSessionProvider extends SessionProvider {
	return null;
	}
	$info['userInfo'] = $userInfo->verified();
	- } elseif ( isset( $info['id'] ) ) { // No point if no session ID
	+ } elseif ( isset( $info['id'] ) ) {
	$info['userInfo'] = $userInfo;
	+ } else {
	+ // No point in returning, loadSessionInfoFromStore() will
	+ // reject it anyway.
	+ return null;
	}
	- }
	-
	- if ( !$info ) {
	+ } elseif ( isset( $info['id'] ) ) {
	+ // No UserID cookie, so insist that the session is anonymous.
	+ $info['userInfo'] = UserInfo::newAnonymous();
	+ } else {
	+ // No session ID and no user is the same as an empty session, so
	+ // there's no point.
	return null;
	}

	- $info += array(
	- 'provider' => $this,
	- 'persisted' => isset( $info['id'] ),
	- 'forceHTTPS' => $this->getCookie( $request, 'forceHTTPS', '', false )
	- );
	-
	return new SessionInfo( $this->priority, $info );
	}

	--
	2.7.0

File Metadata

Mime Type: text/x-diff
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 3301683
Default Alt Text: Close a loophole in CookieSessionProvider.patch (2 KB)

Close a loophole in CookieSessionProvider.patchNo OneActions

Close a loophole in CookieSessionProvider.patchView Options

File Metadata

Event Timeline

Close a loophole in CookieSessionProvider.patch
No One
Actions

Close a loophole in CookieSessionProvider.patch
View Options