Gotcha - 0 Total Deposits means that no initiative can be removed

Impact

The condition to unregister an initiative is the following:

https://github.com/liquity/V2-gov/blob/fc5bff2bd1a650b6dcb68c537b40753d1a59d238/src/Governance.sol#L346-L353

        require(
            (votesForInitiativeSnapshot_.lastCountedEpoch + UNREGISTRATION_AFTER_EPOCHS < currentEpoch)
                || (
                    vetosForInitiative > votesForInitiativeSnapshot_.votes
                        && vetosForInitiative > calculateVotingThreshold() * UNREGISTRATION_THRESHOLD_FACTOR / WAD
                ),
            "Governance: cannot-unregister-initiative"
        );

This means that in theory, if we wait enough epochs, we can kick any initiative

However, there's an edge case in this logic

In the case in which we have 0 total votes, and a few epochs pass, the _snapshotVotesForInitiative will follow this logic path:

        if (initiativeSnapshot.forEpoch < currentEpoch - 1) {
            uint256 votingThreshold = calculateVotingThreshold();
            uint32 start = epochStart();
            uint240 votes =
                lqtyToVotes(initiativeState.voteLQTY, start, initiativeState.averageStakingTimestampVoteLQTY);
            uint240 vetos =
                lqtyToVotes(initiativeState.vetoLQTY, start, initiativeState.averageStakingTimestampVetoLQTY);
            // if the votes didn't meet the voting threshold then no votes qualify
            if (votes >= votingThreshold && votes >= vetos) { /// @audit 0 >= 0
                initiativeSnapshot.votes = uint224(votes);
                initiativeSnapshot.lastCountedEpoch = currentEpoch - 1; /// @audit meaning that the initiative is considered "active"
            } else {
                initiativeSnapshot.votes = 0;
            }
            initiativeSnapshot.forEpoch = currentEpoch - 1;
            votesForInitiativeSnapshot[_initiative] = initiativeSnapshot;
            emit SnapshotVotesForInitiative(_initiative, initiativeSnapshot.votes, initiativeSnapshot.forEpoch);
        }

Which means that the initiative will have it's 0 votes considered as above the votingThreshold

This causes the test to fail due to the call here:

        vm.warp(block.timestamp + governance.EPOCH_DURATION() * 4);
        (v, initData) = governance.snapshotVotesForInitiative(address(maliciousInitiative2));
        assertEq(initData.lastCountedEpoch, currentEpoch - 1, "Epoch Matches"); /// @audit This fails if you have 0 total votes

We would expect the initData.lastCountedEpoch to not increase, but it does due to the edge case

POC

forge test --match-test test_gotcha_0_amt_means_no_removal -vvvv

Fails with:

[FAIL. Reason: Epoch Matches: 13 != 9] test_gotcha_0_amt_means_no_removal() (gas: 818510)

// forge test --match-test test_gotcha_0_amt_means_no_removal -vv
    // All calls should never revert due to malicious initiative
    function test_gotcha_0_amt_means_no_removal() public {
        uint256 zeroSnapshot = vm.snapshot();
        uint256 timeIncrease = 86400 * 30;
        vm.warp(block.timestamp + timeIncrease);

        vm.startPrank(user);

        // should not revert if the user doesn't have a UserProxy deployed yet
        address userProxy = governance.deriveUserProxyAddress(user);
        lqty.approve(address(userProxy), 1e18);

        // deploy and deposit 1 LQTY
        governance.depositLQTY(1e18);
        assertEq(UserProxy(payable(userProxy)).staked(), 1e18);
        (uint88 allocatedLQTY, uint32 averageStakingTimestamp) = governance.userStates(user);
        assertEq(allocatedLQTY, 0);
        // first deposit should have an averageStakingTimestamp if block.timestamp
        assertEq(averageStakingTimestamp, block.timestamp);
        vm.warp(block.timestamp + timeIncrease);
        vm.stopPrank();

        vm.startPrank(lusdHolder);
        lusd.transfer(address(governance), 10000e18);
        vm.stopPrank();

        address maliciousWhale = address(0xb4d);
        deal(address(lusd), maliciousWhale, 2000e18);
        vm.startPrank(maliciousWhale);
        lusd.approve(address(governance), type(uint256).max);

        governance.registerInitiative(address(maliciousInitiative2));
        vm.warp(block.timestamp + governance.EPOCH_DURATION());
        vm.stopPrank();

        // Allocate (Non zero)
        vm.startPrank(user);
        address[] memory initiatives = new address[](1);
        initiatives[0] = address(maliciousInitiative2);
        int176[] memory deltaVoteLQTY = new int176[](1);
        deltaVoteLQTY[0] = 5e17;
        int176[] memory deltaVetoLQTY = new int176[](1);
        governance.allocateLQTY(initiatives, deltaVoteLQTY, deltaVetoLQTY);

        // Remove allocation
        initiatives = new address[](1);
        initiatives[0] = address(maliciousInitiative2);
        deltaVoteLQTY = new int176[](1);
        deltaVoteLQTY[0] = -5e17;
        deltaVetoLQTY = new int176[](1);
        governance.allocateLQTY(initiatives, deltaVoteLQTY, deltaVetoLQTY);

        (Governance.VoteSnapshot memory v, Governance.InitiativeVoteSnapshot memory initData) = governance.snapshotVotesForInitiative(address(maliciousInitiative2));
        uint256 currentEpoch = governance.epoch();
        assertEq(initData.lastCountedEpoch, currentEpoch - 1, "Epoch Matches");

        // Inactive for 4 epochs
        // Add another proposal

        vm.warp(block.timestamp + governance.EPOCH_DURATION() * 4);
        (v, initData) = governance.snapshotVotesForInitiative(address(maliciousInitiative2));
        assertEq(initData.lastCountedEpoch, currentEpoch - 1, "Epoch Matches"); /// @audit This fails if you have 0 total votes

        uint256 unregisterSnapshot = vm.snapshot();
        governance.unregisterInitiative(address(maliciousInitiative2));

    }

More info for POC

For full repro, use: https://github.com/GalloDaSballo/V2-gov/tree/feat-tinfoil

Add the test to GovernanceAttacks.t

liquity / V2-gov

Gotcha - 0 Total Deposits means that no initiative can be removed #14

Impact

POC

More info for POC