Skip to content

Commit 7c3d8ae

Browse files
committed
fix: hardened user API
1 parent 4c7e3f8 commit 7c3d8ae

2 files changed

Lines changed: 31 additions & 0 deletions

File tree

phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/UserController.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,13 @@ public function addUser(Request $request): JsonResponse
370370
$userPasswordConfirm = Filter::filterVar($data->passwordConfirm, FILTER_SANITIZE_SPECIAL_CHARS);
371371
$userIsSuperAdmin = Filter::filterVar($data->isSuperAdmin, FILTER_VALIDATE_BOOLEAN);
372372

373+
// Only SuperAdmins may grant the SuperAdmin flag. Reject the request when a
374+
// non-SuperAdmin attempts to set it, to prevent privilege escalation through
375+
// mass-assignment of is_superadmin on user creation.
376+
if (!$this->currentUser->isSuperAdmin() && (bool) $userIsSuperAdmin) {
377+
return $this->json(['error' => Translation::get(key: 'msgNoPermission')], Response::HTTP_FORBIDDEN);
378+
}
379+
373380
$newUser = new User($this->configuration);
374381

375382
if (!$newUser->isValidLogin($userName)) {

tests/phpMyFAQ/Controller/Administration/Api/UserControllerTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,4 +243,28 @@ public function testUpdateRightsNonSuperAdminCannotGrantRightTheyDoNotHold(): vo
243243

244244
$this->assertSame(Response::HTTP_FORBIDDEN, $response->getStatusCode());
245245
}
246+
247+
public function testAddUserNonSuperAdminCannotGrantSuperAdminFlag(): void
248+
{
249+
$session = new Session(new MockArraySessionStorage());
250+
// Acting user holds USER_ADD/EDIT/DELETE but is NOT a SuperAdmin.
251+
$actingUser = $this->buildActingUser(userId: 5, isSuperAdmin: false);
252+
$controller = $this->buildController($session, $actingUser);
253+
$csrf = $this->primeCsrf($session, 'add-user');
254+
255+
$request = $this->jsonRequest([
256+
'csrf' => $csrf,
257+
'userName' => 'evil_superadmin',
258+
'realName' => 'Evil SA',
259+
'email' => 'evil@example.test',
260+
'automaticPassword' => false,
261+
'password' => 'Sup3rSecret!42',
262+
'passwordConfirm' => 'Sup3rSecret!42',
263+
'isSuperAdmin' => true, // privilege escalation attempt
264+
]);
265+
266+
$response = $controller->addUser($request);
267+
268+
$this->assertSame(Response::HTTP_FORBIDDEN, $response->getStatusCode());
269+
}
246270
}

0 commit comments

Comments
 (0)