From 377e2e34cecf7c6fa1c7a75d697b6f9c68bcb5b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 3 Nov 2026 12:41:10 +0100 Subject: [PATCH 1/2] fix: Reject login with NUL bytes rather than converting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Côme Chilliet --- lib/private/User/Manager.php | 7 ++++++- tests/lib/User/ManagerTest.php | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 9a8fb74a03d2b..7cc91748a5137 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -240,7 +240,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 b1585dd3259f7..a78786ca9992a 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -186,6 +186,26 @@ public function testCheckPasswordNotSupported(): void { $this->assertFalse($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()) From 91424e2793a4185c55f89765fb6fbc0157e1ae29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 4 Nov 2026 08:36:38 +0100 Subject: [PATCH 2/2] chore(tests): Fix user manager tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/User/ManagerTest.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index a78786ca9992a..688169787cb16 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -63,7 +63,6 @@ public function testGetBackends(): void { $this->assertEquals([$userDummyBackend, $dummyDatabaseBackend], $manager->getBackends()); } - public function testUserExistsSingleBackendExists(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) @@ -201,9 +200,10 @@ public function testCheckPasswordNullByteInLoginName(): void { } }); - $this->manager->registerBackend($backend); + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager->registerBackend($backend); - $this->assertFalse($this->manager->checkPassword("foo\0", 'bar')); + $this->assertFalse($manager->checkPassword("foo\0", 'bar')); } public function testGetOneBackendExists(): void { @@ -350,7 +350,6 @@ public function testCreateUserInvalid($uid, $password, $exception): void { ->with(\OC\User\Backend::CREATE_USER) ->willReturn(true); - $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend); @@ -382,7 +381,6 @@ public function testCreateUserSingleBackendNotExists(): void { $this->assertEquals('foo', $user->getUID()); } - public function testCreateUserSingleBackendExists(): void { $this->expectException(\Exception::class); @@ -429,7 +427,6 @@ public function testCreateUserNoBackends(): void { $this->assertFalse($manager->createUser('foo', 'bar')); } - public function testCreateUserFromBackendWithBackendError(): void { $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('Could not create account'); @@ -446,7 +443,6 @@ public function testCreateUserFromBackendWithBackendError(): void { $manager->createUserFromBackend('MyUid', 'MyPassword', $backend); } - public function testCreateUserTwoBackendExists(): void { $this->expectException(\Exception::class);