UploadedFilesController::destroy() (web) and Api\UploadedFilesController::destroy() (API) both called Storage::delete() without checking the return value, then created an upload deleted action-log entry and returned a success response unconditionally. HasUploads::uploads() excludes files whose filename matches an upload deleted log for the same object, so a silently-failed physical delete produced a state where:

• The bytes remain on disk (nobody removed them).
• An action log states the file has been deleted.
• The admin sees a "success" response to the delete request.
• The attachment no longer appears in the normal UI listing (HasUploads hides it).

An administrator asked to purge a sensitive attachment (a signed contract that needs to disappear, an old signature block, a document the data subject requested erased) is told the deletion succeeded while the bytes are still on disk and are still accessible by anyone with direct filesystem or backup access.

Same family as GHSA-v37p-hr9x-5w85 (image write-fail on ImageUploadRequest) and GHSA- (acceptance evidence write-fail), also reported by Christopher Finks. Scoped this time to attachment deletion rather than image write or evidence write.

Severity

Medium. CVSS 3.1: 4.3

CVSS:3.1/AV:N/AC:L/PR:H/UI:N/S:U/C:L/I:L/A:N

• Privileges Required High. Deleting attachments requires manageFiles on the parent object, which is an admin-adjacent permission.
• Confidentiality Low. The retained data was already accessible to the admin who initiated the (failed) delete; the disclosure is downstream: backup archives, filesystem access, and any future privileged reader all still see the "deleted" attachment.
• Integrity Low. The action log claims a deletion happened that did not. The HasUploads::uploads listing is inconsistent with actual disk contents.
• Availability None.

The rating is Medium rather than Low because the primary use case for attachment deletion in Snipe-IT is intentional purge of sensitive documents (contract cancellations, DSAR requests, PII removal). A silently-broken purge failure that presents as success actively defeats the compliance workflow it was doing.

Weakness

• CWE-754 (Improper Check for Unusual or Exceptional Conditions) primary
• CWE-459 (Incomplete Cleanup) as alternative framing
• CWE-212 (Improper Removal of Sensitive Information Before Storage or Transfer) as an alternative for the compliance angle

Affected Versions

<= 8.6.3 and all develop commits pre-fix. The unchecked Storage::delete() pattern has existed in both controllers since attachment upload was introduced.

Attack Chain

Passive-failure bug rather than an attacker-driven chain. The vulnerability manifests when:

1. The install runs on a filesystem driver that returns false on delete failure (default local disk with read-only file, S3 with revoked delete permission, GCS quota, etc.).
2. An admin performs an attachment delete through the web UI (DELETE /{object_type}/{id}/files/{file_id}/delete) or through the API (DELETE /api/v1/{object_type}/{id}/files/{file_id}).
3. The physical delete silently fails.

Observed state after: file still on disk, upload deleted action-log row present, HasUploads listing hides the file, admin's UI shows "success".

Root Cause

app/Http/Controllers/UploadedFilesController.php (pre-fix) and app/Http/Controllers/Api/UploadedFilesController.php (pre-fix), both in the destroy() method:

if ($log) {
    if (Storage::exists(...$path)) {
        Storage::delete(...$path);   // return value discarded
    }
    if ($log->logUploadDelete($object, $log->filename)) {   // fires regardless
        return ...->with('success', ...);
    }
}

HasUploads::uploads() at app/Models/Traits/HasUploads.php: filters out uploaded action logs whose filename matches an upload deleted log for the same object. The exclusion is what hides the file from the UI even though the bytes are still present.

Fix

Both controllers now check Storage::delete()'s return value. If it returns false, the flow refuses to write the upload deleted action log and returns a translated error response instead of the success response. logUploadDelete() no longer fires when the physical delete failed, so HasUploads::uploads() continues to surface the row and the operator can see that the attachment was not actually removed.

The web path returns a redirect back with error flash + trans('general.file_upload_status.delete.error', 1). The API path returns HTTP 500 with the same translated message.

Fix Commit

5d36aef

Regression Tests

tests/Feature/FileUploads/UploadDeleteStorageFailureTest.php:

• test_web_delete_does_not_log_deletion_when_physical_delete_fails uploads a real attachment, mocks Storage::delete to return false, sends the web delete request, asserts the response flashes an error, and asserts no upload deleted action-log row was created.
• test_api_delete_does_not_log_deletion_when_physical_delete_fails covers the API endpoint. Asserts HTTP 500 and the same absent-log assertion.

Credit

Christopher Finks - Constraint Layer (christopherfi-dev)