From 8c77e32f756be4e5e53de1c33b8825773166a72d Mon Sep 17 00:00:00 2001
From: John Kennedy <65985482+jkennedyvz@users.noreply.github.com>
Date: Sun, 8 Feb 2026 21:44:19 -0800
Subject: [PATCH] Fix TOCTOU and improve error handling in evidence deletion

The deleteButtonClicked function had multiple issues:
1. TOCTOU race condition - checked file existence implicitly via
   QFile::remove return value, then deleted DB entry separately
2. Incomplete error handling - only reported file deletion failures,
   not database deletion failures
3. Inconsistent state - database entry deleted even if file deletion
   failed, leading to orphaned files

Changes:
- Perform both operations atomically (no race window)
- Store results of both file and database deletion
- Provide specific error messages for each failure scenario:
  * Both operations fail
  * Only file deletion fails (DB entry removed)
  * Only DB deletion fails (file removed)
- Only close dialog if both operations succeed
- Better UX with clear error messages and next steps

This eliminates the TOCTOU vulnerability and prevents inconsistent
state between filesystem and database.
---
 src/forms/getinfo/getinfo.cpp | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/forms/getinfo/getinfo.cpp b/src/forms/getinfo/getinfo.cpp
index 5c7497e5..1a1b5e08 100644
--- a/src/forms/getinfo/getinfo.cpp
+++ b/src/forms/getinfo/getinfo.cpp
@@ -104,21 +104,32 @@ void GetInfo::deleteButtonClicked() {
 
   if (reply == QMessageBox::Yes) {
     Q_EMIT  setActionButtonsEnabled(false);
-    bool shouldClose = true;
 
     model::Evidence evi = evidenceEditor->encodeEvidence();
-    if (!QFile::remove(evi.path)) {
+    bool fileDeleted = QFile::remove(evi.path);
+    bool dbDeleted = db->deleteEvidence(evidenceID);
+
+    if (!fileDeleted && !dbDeleted) {
       QMessageBox::warning(this, tr("Could not delete"),
-                           tr("Unable to delete evidence file.\n"
+                           tr("Unable to delete evidence file and database entry.\n"
                            "You can try deleting the file directly. File Location:\n%1")
                              .arg(evi.path));
-      shouldClose = false;
     }
-
-    db->deleteEvidence(evidenceID);
+    else if (!fileDeleted) {
+      QMessageBox::warning(this, tr("Could not delete file"),
+                           tr("Unable to delete evidence file (database entry removed).\n"
+                           "You can try deleting the file directly. File Location:\n%1")
+                             .arg(evi.path));
+    }
+    else if (!dbDeleted) {
+      QMessageBox::warning(this, tr("Could not delete database entry"),
+                           tr("Unable to delete database entry (file removed).\n"
+                           "The database may be corrupted. Error: %1")
+                             .arg(db->errorString()));
+    }
 
     Q_EMIT setActionButtonsEnabled(true);
-    if (shouldClose) {
+    if (fileDeleted && dbDeleted) {
       close();
     }
   }