diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 8eacba32c4ad1..df38a336be236 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -245,7 +245,12 @@ public function checkPassword($loginName, $password) { * @return IUser|false the User object on success, false otherwise */ public function checkPasswordNoLogging($loginName, $password) { - $loginName = str_replace("\0", '', $loginName); + // A NUL byte in the login name must not turn it into a second name for an + // account: the checks done before the password check (e.g. 2FA enforcement + // in Session::logClientIn()) see the name as sent, so the name has to stay the same + if (str_contains($loginName, "\0")) { + return false; + } $password = str_replace("\0", '', $password); $cachedBackend = $this->cache->get($loginName); diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index ac4a354aa0333..5936a078fa3c7 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -175,6 +175,26 @@ public function testCheckPasswordNotSupported(): void { $this->assertFalse($this->manager->checkPassword('foo', 'bar')); } + public function testCheckPasswordNullByteInLoginName(): void { + $backend = $this->createMock(\Test\Util\User\Dummy::class); + $backend->expects($this->never()) + ->method('checkPassword'); + + $backend->expects($this->any()) + ->method('implementsActions') + ->willReturnCallback(function ($actions) { + if ($actions === BACKEND::CHECK_PASSWORD) { + return true; + } else { + return false; + } + }); + + $this->manager->registerBackend($backend); + + $this->assertFalse($this->manager->checkPassword("foo\0", 'bar')); + } + public function testGetOneBackendExists(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once())