jenkins-bot has submitted this change and it was merged.
Change subject: Close a loophole in CookieSessionProvider
..
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
---
M includes/session/CookieSessionProvider.php
M tests/phpunit/includes/session/CookieSessionProviderTest.php
2 files changed, 18 insertions(+), 12 deletions(-)
Approvals:
BryanDavis: Looks good to me, approved
jenkins-bot: Verified
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 @@
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,20 +131,21 @@
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 );
}
diff --git a/tests/phpunit/includes/session/CookieSessionProviderTest.php
b/tests/phpunit/includes/session/CookieSessionProviderTest.php
index 2b7e0a1..e5df458 100644
--- a/tests/phpunit/includes/session/CookieSessionProviderTest.php
+++ b/tests/phpunit/includes/session/CookieSessionProviderTest.php
@@ -184,7 +184,9 @@
$this->assertNotNull( $info );
$this->assertSame( $params['priority'], $info->getPriority() );
$this->assertSame( $sessionId, $info->getId() );
- $this->assertNull( $info->getUserInfo() );
+ $this->assertNotNull( $info->getUserInfo() );
+ $this->assertSame( 0, $info->getUserInfo()->getId() );
+ $this->assertNull( $info->getUserInfo()->getName() );
$this->assertFalse( $info->forceHTTPS() );
// User, no session key
--
To view, visit https://gerrit.wikimedia.org/r/267521
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I44096c69aa7bd285e4e2472959e8d892200c5f2c
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie
Gerrit-Reviewer: BryanDavis
Gerrit-Reviewer: jenkins-bot <>
___
MediaWiki-commits mailing list