Anomie has uploaded a new change for review. https://gerrit.wikimedia.org/r/267521
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 1 file changed, 15 insertions(+), 11 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/21/267521/1 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 ); } -- To view, visit https://gerrit.wikimedia.org/r/267521 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I44096c69aa7bd285e4e2472959e8d892200c5f2c Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Anomie <bjor...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits