From 1e3d2f6ed2159e304250b0ba7e74ccd31c6bd454 Mon Sep 17 00:00:00 2001
From: ArokyaMatthew <arokyamatthewnathan@gmail.com>
Date: Sun, 5 Apr 2026 07:08:56 +0530
Subject: [PATCH] =?UTF-8?q?[UTXO-BUG]=20HIGH-1:=20TOCTOU=20race=20conditio?=
 =?UTF-8?q?n=20in=20spend=5Fbox()=20=E2=80=94=20silent=20double-spend?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

spend_box() performs SELECT then UPDATE without an exclusive lock when
called standalone (own=True). Two concurrent callers can both read
spent_at IS NULL, then both UPDATE — the second gets rowcount=0 but
the method never checks rowcount, so it returns the box dict as if
the spend succeeded. The caller proceeds thinking the box was spent.

Fixes:
1. Acquire BEGIN IMMEDIATE when own=True to serialize the operation
2. Check UPDATE rowcount == 1; raise ValueError if 0 (concurrent spend)
3. Proper ROLLBACK on all early-exit paths

Tests added:
- test_spend_box_double_spend_raises
- test_spend_box_nonexistent_returns_none

All 36 tests pass.

Bounty: #2819 (High, 100 RTC)
---
 node/test_utxo_db.py | 20 ++++++++++++++++++++
 node/utxo_db.py      | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/node/test_utxo_db.py b/node/test_utxo_db.py
index 78352f80..aceac907 100644
--- a/node/test_utxo_db.py
+++ b/node/test_utxo_db.py
@@ -181,6 +181,26 @@ def test_double_spend_rejected(self):
         self.assertEqual(self.db.get_balance('bob'), 100 * UNIT)
         self.assertEqual(self.db.get_balance('eve'), 0)
 
+    def test_spend_box_double_spend_raises(self):
+        """spend_box() must raise ValueError on double-spend, not silently
+        return the box dict (bounty #2819 HIGH-1 TOCTOU fix)."""
+        self._apply_coinbase('alice', 100 * UNIT)
+        boxes = self.db.get_unspent_for_address('alice')
+        box_id = boxes[0]['box_id']
+
+        # First spend succeeds
+        result = self.db.spend_box(box_id, 'tx_first')
+        self.assertIsNotNone(result)
+
+        # Second spend must raise, not return silently
+        with self.assertRaises(ValueError):
+            self.db.spend_box(box_id, 'tx_second')
+
+    def test_spend_box_nonexistent_returns_none(self):
+        """spend_box() on a nonexistent box_id returns None."""
+        result = self.db.spend_box('deadbeef' * 8, 'tx_whatever')
+        self.assertIsNone(result)
+
     def test_nonexistent_input_rejected(self):
         ok = self.db.apply_transaction({
             'tx_type': 'transfer',
diff --git a/node/utxo_db.py b/node/utxo_db.py
index db77aaf1..94f3c980 100644
--- a/node/utxo_db.py
+++ b/node/utxo_db.py
@@ -207,34 +207,63 @@ def spend_box(self, box_id: str, spent_by_tx: str,
         """
         Mark a box as spent.  Returns the box dict or None if not found.
         Raises ValueError on double-spend attempt.
+
+        When called without an external ``conn``, acquires BEGIN IMMEDIATE
+        to prevent TOCTOU races between the SELECT and UPDATE.
         """
         own = conn is None
         if own:
             conn = self._conn()
         try:
+            if own:
+                conn.execute("BEGIN IMMEDIATE")
+
             row = conn.execute(
                 "SELECT * FROM utxo_boxes WHERE box_id = ?", (box_id,)
             ).fetchone()
             if not row:
+                if own:
+                    conn.execute("ROLLBACK")
                 return None
             if row['spent_at'] is not None:
+                if own:
+                    conn.execute("ROLLBACK")
                 raise ValueError(
                     f"Double-spend attempt: box {box_id[:16]} already spent "
                     f"by tx {row['spent_by_tx'][:16]}"
                 )
-            conn.execute(
+            updated = conn.execute(
                 """UPDATE utxo_boxes
                    SET spent_at = ?, spent_by_tx = ?
                    WHERE box_id = ? AND spent_at IS NULL""",
                 (int(time.time()), spent_by_tx, box_id),
-            )
+            ).rowcount
+            if updated != 1:
+                # Another connection spent this box between our SELECT
+                # and UPDATE — treat as double-spend.
+                if own:
+                    conn.execute("ROLLBACK")
+                raise ValueError(
+                    f"Double-spend race: box {box_id[:16]} was spent "
+                    f"concurrently"
+                )
             if own:
                 conn.commit()
             return dict(row)
+        except ValueError:
+            raise
+        except Exception:
+            if own:
+                try:
+                    conn.execute("ROLLBACK")
+                except Exception:
+                    pass
+            raise
         finally:
             if own:
                 conn.close()
 
+
     def get_box(self, box_id: str) -> Optional[dict]:
         """Get a box by ID (spent or unspent)."""
         conn = self._conn()