Admidio: Any logged-in user can delete inventory fields via mode=field_delete — incomplete fix of #2024
Summary
Commit d37ca6b27b9674238e58491cf7ba292e66898f15 ("Delete item not check admin rights #2024", 2026-04-12) added a missing isAdministratorInventory() gate to case 'item_delete': in modules/inventory.php. The same fix was not applied to the sibling case 'field_delete': handler, which destroys an entire inventory field definition, cascading to every adm_inventory_item_data row that referenced that field and every adm_inventory_field_options entry. The handler validates only a session-bound CSRF token; there is no isAdministratorInventory() check at the controller level, and Admidio\Inventory\Entity\ItemField::delete() does not enforce one at the entity level either (unlike its sibling ItemField::save(), which does check $gCurrentUser->isAdministrator()). Any user who can log in to the site can permanently destroy a non-system inventory field by sending one POST.
Details
Vulnerable Code
modules/inventory.php mode dispatch at the top of the file:
```php
// modules/inventory.php:64-72 (top-level rights gate)
if ($gSettingsManager->getInt('inventory_module_enabled') === 0) {
throw new Exception('SYS_MODULE_DISABLED');
} elseif ($gSettingsManager->getInt('inventory_module_enabled') === 2 && !$gValidLogin
|| ($gSettingsManager->getInt('inventory_module_enabled') === 3 && !$gCurrentUser->isAdministratorInventory())
|| ($gSettingsManager->getInt('inventory_module_enabled') === 4 && !InventoryPresenter::isCurrentUserKeeper() && !$gCurrentUser->isAdministratorInventory())
|| ($gSettingsManager->getInt('inventory_module_enabled') === 5 && !$gCurrentUser->isAllowedToSeeInventory() && !$gCurrentUser->isAdministratorInventory())) {
throw new Exception('SYS_NO_RIGHTS');
}
```
inventory_module_enabled=2 is the default value (install/db_scripts/preferences.php: 'inventory_module_enabled' => '2',). At this setting the only gate is $gValidLogin — any logged-in user reaches the switch.
modules/inventory.php:123-131 — field_delete only checks the session CSRF, not admin rights:
```php
case 'field_delete':
// check the CSRF token of the form against the session token
SecurityUtils::validateCsrfToken($_POST['adm_csrf_token']);
$itemFieldService = new ItemFieldService($gDb, $getinfUUID);
$itemFieldService->delete();
echo json_encode(array('status' => 'success', 'message' => $gL10n->get('SYS_INVENTORY_ITEMFIELD_DELETED')));
break;
```
SecurityUtils::validateCsrfToken (src/Infrastructure/Utils/SecurityUtils.php) is a session-token compare:
```php
public static function validateCsrfToken(string $csrfToken)
{
global $gCurrentSession;
if ($csrfToken !== $gCurrentSession->getCsrfToken()) {
throw new Exception('Invalid or missing CSRF token!');
}
}
```
The token is the session's CSRF token, which the actor's own session prints on every page (it appears in ?mode=field_list's response in the data-csrf JSON callback). So a non-admin attacker has it for free.
src/Inventory/Service/ItemFieldService.php:46-49 — the service just delegates:
```php
public function delete(): bool
{
return $this->itemFieldRessource->delete();
}
```
src/Inventory/Entity/ItemField.php:54-88 — the entity's delete() blocks system fields via inf_system==1 but otherwise has no isAdministrator() check:
```php
public function delete(): bool
{
global $gCurrentOrgId;
if ($this->getValue('inf_system') == 1) {
// System fields could not be deleted
throw new Exception('Item fields with the flag "system" could not be deleted.');
}
$this->db->startTransaction();
// close gap in sequence
$sql = 'UPDATE ' . TBL_INVENTORY_FIELDS . ' SET inf_sequence = inf_sequence - 1 ...';
$this->db->queryPrepared($sql, ...);
// delete all data of this field in the item data table
$sql = 'DELETE FROM ' . TBL_INVENTORY_ITEM_DATA . ' WHERE ind_inf_id = ? -- $infId';
$this->db->queryPrepared($sql, array($infId));
// delete all data of this field in the field select options table
$sql = 'DELETE FROM ' . TBL_INVENTORY_FIELD_OPTIONS . ' WHERE ifo_inf_id = ? -- $infId';
$this->db->queryPrepared($sql, array($infId));
$return = parent::delete(); // DELETE FROM adm_inventory_fields WHERE inf_id = ?
$this->db->endTransaction();
return $return;
}
```
Compare with ItemField::save() at line 230, which *does* enforce admin:
```php
public function save(bool $updateFingerPrint = true): bool
{
global $gCurrentUser, $gCurrentOrgId;
// only administrators can edit item fields
if (!$gCurrentUser->isAdministrator() && !$this->saveChangesWithoutRights) {
throw new Exception('Item field could not be saved because only administrators are allowed to edit item fields.');
}
...
}
```
The asymmetry is the bug: save is gated, delete is not.
