Proposal Can Be Stuck Indefinitely Due to Lack of Quorum

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L396 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/DAO.sol#L281

Vulnerability details

Impact

Proposals within the system may become indefinitely stagnant if they fail to achieve the required quorum. This issue can lead to several consequences, including:

• Making a new proposal for the same user will be disabled until the proposal achieves quorum (indefinite time)
• Crucial proposals (such as setContracts or sendSalt) becoming unchangeable.
• If the DAO implements changes in the protocol, these active proposals might be exploited by malicious users, especially in sensitive functions like proposeCallContract, giving them undue influence as the DAO makes the call.

The difficulty in achieving a quorum is further compounded as the required quorum can vary from 1x to 3x depending on the proposal type, making it harder for certain proposals to reach completion.

Proof of Concept

The issue becomes apparent when examining the finalizeBallot function. The primary requirement for finalizing a ballot is:

require(proposals.canFinalizeBallot(ballotID), "The ballot is not yet able to be finalized");

Within this requirement, several checks are performed:

function canFinalizeBallot(uint256 ballotID) external view returns (bool) {
        Ballot memory ballot = ballots[ballotID];
        if (!ballot.ballotIsLive) {
            return false;
        }

        // Check that the minimum duration has passed
        if (block.timestamp < ballot.ballotMinimumEndTime) {
            return false;
        }

        // Check that the required quorum has been reached
@>       if (totalVotesCastForBallot(ballotID) < requiredQuorumForBallotType(ballot.ballotType)) {
            return false;
        } 

        return true;
    }

./src/dao/Proposals.sol

A proposal can be stuck indefinitely if it doesn’t meet the requiredQuorumForBallotType. As upKeep is called, the total supply increases, making it increasingly difficult to achieve the quorum over time.

This test case illustrates the issue:

    function testCanFinalizeBallot() public {
        string memory ballotName = "parameter:2";

        uint256 initialStake = 10000000 ether;

        vm.startPrank(alice);
        staking.stakeSALT(1110111 ether);
        proposals.proposeParameterBallot(2, "description");
        staking.unstake(1110111 ether, 2);
        uint256 ballotID = proposals.openBallotsByName(ballotName);

        // Early ballot, no quorum
        bool canFinalizeBallotStillEarly = proposals.canFinalizeBallot(ballotID);

        // Ballot reached end time, no quorum
        vm.warp(block.timestamp + daoConfig.ballotMinimumDuration() + 1); // ballot end time reached

        vm.expectRevert("SALT staked cannot be zero to determine quorum");
        proposals.canFinalizeBallot(ballotID);
        vm.stopPrank();

        vm.prank(DEPLOYER);
        staking.stakeSALT(initialStake);

        bool canFinalizeBallotPastEndtime = proposals.canFinalizeBallot(ballotID);

        // Almost reach quorum
        vm.prank(alice);
        staking.stakeSALT(1110111 ether);

        // Default user has no access to the exchange, but can still vote
        vm.prank(DEPLOYER);
        salt.transfer(address(this), 1000 ether);

        salt.approve(address(staking), type(uint256).max);
        staking.stakeSALT(1000 ether);
        proposals.castVote(ballotID, Vote.INCREASE);

        vm.startPrank(alice);
        proposals.castVote(ballotID, Vote.INCREASE);

        bool canFinalizeBallotAlmostAtQuorum = proposals.canFinalizeBallot(ballotID);

        // Reach quorum
        staking.stakeSALT(1 ether);

        // Recast vote to include new stake
        proposals.castVote(ballotID, Vote.DECREASE);

        bool canFinalizeBallotAtQuorum = proposals.canFinalizeBallot(ballotID);

        // Assert
        assertEq(canFinalizeBallotStillEarly, false, "Should not be able to finalize live ballot");
        assertEq(canFinalizeBallotPastEndtime, false, "Should not be able to finalize non-quorum ballot");
        assertEq(
            canFinalizeBallotAlmostAtQuorum,
            false,
            "Should not be able to finalize ballot if quorum is just beyond the minimum "
        );
        assertEq(
            canFinalizeBallotAtQuorum,
            true,
            "Should be able to finalize ballot if quorum is reached and past the minimum end time"
        );
    }

./src/dao/tests/Proposals.t.sol

Tools Used

Manual code review

Recommended Mitigation Steps

Add expiration time, if the ballot dot pass more thant ballotMinimumEndTime but the requiredQuorumForBallotType is not enough may close this proposal wiouth change and give the oportunitie to the user can open a new proposal.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #362

[c4-judge]

Picodes marked the issue as satisfactory