# MaxHeap.sol: Already extracted tokenId may be extracted again

MaxHeap.extractMax leaves stale heap[size] value; maxHeapify reads out-of-bounds index, re-extracting an already-removed tokenId with wrong value.

- Payout: Contest reward
- Severity: medium
- Program: Collective
- Credited: DanielArmstrong
- Disclosed: 2024-02-08
- Write-up quality: 6/10
- Original: https://github.com/code-423n4/2023-12-revolutionprotocol-findings/issues/266
- Library page: https://bugbountylibrary.com/r/c4%3A2023-12-revolutionprotocol%3AM-08
- Tags: domain:web3 impact:integrity kind:contest platform:code4rena sev:medium tech:solidity technique:source-review vuln:business-logic

**Target:** MaxHeap.sol heap data structure (Solidity)
**Root cause:** extractMax decrements size but never zeroes heap[size]; maxHeapify reads heap[left]/heap[right] before the pos>=size/2 bounds check, so a stale out-of-bounds value participates in comparisons.
**Steps:**
1. Insert tokenIds 1,2,3 with value 20; extractMax three times (heap now logically empty but heap[0..2] retain stale ids).
2. Insert tokenIds 4,5 with value 10.
3. Call updateValue(4,5) to trigger maxHeapify.
4. maxHeapify reads heap[right]=heap[size] stale value 3/20, swaps it up.
5. extractMax returns (3,20) instead of (5,10) — already-extracted tokenId re-extracted.
**How found:** Code reading of extractMax vs maxHeapify: noticed size decrement without clearing heap[size], and that maxHeapify reads left/right values before the bounds check, so out-of-bounds dirty slot is consumed.
**Exploitation:** Admin-only extractMax; requires values to decrease (updateValue), which CultureIndex (upvote-only) does not permit, so no live exploit path — spec violation only.
**Insight:** In array-backed heaps, always zero the vacated slot on pop and place the bounds check before any child-value reads — reading heap[left]/heap[right] prior to the size guard lets stale slots leak into comparisons.