Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F3294329
[Obsolete] Close a loophole in CookieSessionProvider.patch
No One
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Authored By
Anomie
Jan 30 2016, 4:08 PM
2016-01-30 16:08:02 (UTC+0)
Size
2 KB
Referenced Files
None
Subscribers
None
[Obsolete] Close a loophole in CookieSessionProvider.patch
View Options
From 5fcb594bcb35b882b88ddc88d7a9ff62eb3d8644 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..5771588 100644
--- a/includes/session/CookieSessionProvider.php
+++ b/includes/session/CookieSessionProvider.php
@@ -104,7 +104,10 @@ 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,
+ 'persisted' => isset( $info['id'] ),
+ 'forceHTTPS' => $this->getCookie( $request, 'forceHTTPS', '', false )
);
if ( !SessionManager::validateSessionId( $info['id'] ) ) {
unset( $info['id'] );
@@ -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
Details
Attached
Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
3301676
Default Alt Text
[Obsolete] Close a loophole in CookieSessionProvider.patch (2 KB)
Attached To
Mode
T125283: Users occasionally logged in as different users after SessionManager deployment
Attached
Detach File
Event Timeline
Anomie
updated the name for this file from "
Close a loophole in CookieSessionProvider.patch
" to "
[Obsolete] Close a loophole in CookieSessionProvider.patch
".
Jan 30 2016, 4:11 PM
2016-01-30 16:11:17 (UTC+0)
Log In to Comment