SALT staker can get extra voting power by simply unstaking their xSALT

mediumCode4rena

Lines of code

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

Vulnerability details

Impact

SALT staker might have chance to finalize the vote by manipulating the required quorum

Proof of Concept

When a proposal is created, anyone can vote by calling Proposals#castVote() as far as they have voting power. Anyone can finalize the vote on a specific ballot by calling DAO#finalizeBallot() as soon as the ballot meet below requirements:

• The ballot is live
• ballotMinimumEndTime(the minimum duration) has passed
• The required quorum has been reached

If the total number of votes is less than the required quorum, the ballot can not be finalized:

solidity📋
396:  if ( totalVotesCastForBallot(ballotID) < requiredQuorumForBallotType( ballot.ballotType ))
397:    return false;

The required quorum varies based on the ballot type and the total staked SALT:

solidity📋
  function requiredQuorumForBallotType( BallotType ballotType ) public view returns (uint256 requiredQuorum)
  {
    // The quorum will be specified as a percentage of the total amount of SALT staked
    uint256 totalStaked = staking.totalShares( PoolUtils.STAKED_SALT );
    require( totalStaked != 0, "SALT staked cannot be zero to determine quorum" );
    
    if ( ballotType == BallotType.PARAMETER )
      requiredQuorum = ( 1 * totalStaked * daoConfig.baseBallotQuorumPercentTimes1000()) / ( 100 * 1000 );
    else if ( ( ballotType == BallotType.WHITELIST_TOKEN ) || ( ballotType == BallotType.UNWHITELIST_TOKEN ) )
      requiredQuorum = ( 2 * totalStaked * daoConfig.baseBallotQuorumPercentTimes1000()) / ( 100 * 1000 );
    else
    // All other ballot types require 3x multiple of the baseQuorum
      requiredQuorum = ( 3 * totalStaked * daoConfig.baseBallotQuorumPercentTimes1000()) / ( 100 * 1000 );
    
    // Make sure that the requiredQuorum is at least 0.50% of the total SALT supply.
    // Circulating supply after the first 45 days of emissions will be about 3 million - so this would require about 16% of the circulating
    // SALT to be staked and voting to pass a proposal (including whitelisting) 45 days after deployment..
    uint256 totalSupply = ERC20(address(exchangeConfig.salt())).totalSupply();
    uint256 minimumQuorum = totalSupply * 5 / 1000;
    
    if ( requiredQuorum < minimumQuorum )
      requiredQuorum = minimumQuorum;
  }

If a SALT staker unstakes their xSALT, the required quorum will decrease. However, this action does not impact the voting number the staker has casted. A staker might have chance to finalize the ballot in advance by unstaking their xSALT if the total number of votes and required quorum are close enough. Once the ballot is closed, the staker can simply cancel their unstaking without suffering any loss.

Copy below codes to Proposals.t.sol and run COVERAGE="yes" NETWORK="sep" forge test -vv --rpc-url RPC_URL --match-test testCanFinalizeBallotByUnstaking

solidity📋
  function testCanFinalizeBallotByUnstaking() public {
    uint256 initialStake = 10000000 ether;
    vm.prank(DEPLOYER);
    staking.stakeSALT( initialStake );
    
    vm.startPrank(alice);
    staking.stakeSALT(1110111 ether);
    uint256 ballotID = proposals.proposeParameterBallot(2, "description" );
    proposals.castVote(ballotID, Vote.INCREASE);
    vm.stopPrank();
    
    vm.warp(block.timestamp + daoConfig.ballotMinimumDuration() + 1); // ballot end time reached
    
    // Reach quorum
    vm.startPrank(alice);
    //@audit-info before unstaking, the ballot can not be finalized
    bool canFinalizeBeforeUnstaking = proposals.canFinalizeBallot(ballotID);
    assertEq(canFinalizeBeforeUnstaking, false);
    //@audit-info alice unstake SALT
    staking.unstake(1110111 ether, 52);
    //@audit-info after unstaking, the ballot can be finalized now. 
    bool canFinalizeAfterUnstaking = proposals.canFinalizeBallot(ballotID);
    assertEq(canFinalizeAfterUnstaking, true);
    vm.stopPrank();
  }

From above codes we can see, the ballot can be finalized after alice unstakes all her xSALT.

Tools Used

Manual review

Recommended Mitigation Steps

There are several ways to fix this problem. The simplest one is calculating and storing the required quorum when the proposal is created. Once the required quorum is fixed, no one can change it by unstaking their xSALT:

diff📋
  struct Ballot
  {
    uint256 ballotID;
    bool ballotIsLive;
    
    BallotType ballotType;
    string ballotName;
    address address1;
    uint256 number1;
    string string1;
    string description;
    
    // The earliest timestamp at which a ballot can end. Can be open longer if the quorum has not yet been reached for instance.
    uint256 ballotMinimumEndTime;
+   uint256 requiredQuorum;
  }

  function _possiblyCreateProposal( string memory ballotName, BallotType ballotType, address address1, uint256 number1, string memory string1, string memory string2 ) internal returns (uint256 ballotID)
  {
    require( block.timestamp >= firstPossibleProposalTimestamp, "Cannot propose ballots within the first 45 days of deployment" );
    
    // The DAO can create confirmation proposals which won't have the below requirements
    if ( msg.sender != address(exchangeConfig.dao() ) )
    {
      // Make sure that the sender has the minimum amount of xSALT required to make the proposal
      uint256 totalStaked = staking.totalShares(PoolUtils.STAKED_SALT);
      uint256 requiredXSalt = ( totalStaked * daoConfig.requiredProposalPercentStakeTimes1000() ) / ( 100 * 1000 );
      
      require( requiredXSalt > 0, "requiredXSalt cannot be zero" );
      
      uint256 userXSalt = staking.userShareForPool( msg.sender, PoolUtils.STAKED_SALT );
      require( userXSalt >= requiredXSalt, "Sender does not have enough xSALT to make the proposal" );
      
      // Make sure that the user doesn't already have an active proposal
      require( ! _userHasActiveProposal[msg.sender], "Users can only have one active proposal at a time" );
    }
    
    // Make sure that a proposal of the same name is not already open for the ballot
    require( openBallotsByName[ballotName] == 0, "Cannot create a proposal similar to a ballot that is still open" );
    require( openBallotsByName[ string.concat(ballotName, "_confirm")] == 0, "Cannot create a proposal for a ballot with a secondary confirmation" );
    
    uint256 ballotMinimumEndTime = block.timestamp + daoConfig.ballotMinimumDuration();
    
    // Add the new Ballot to storage
    ballotID = nextBallotID++;
-   ballots[ballotID] = Ballot( ballotID, true, ballotType, ballotName, address1, number1, string1, string2, ballotMinimumEndTime );
+   uint requiredQuorum = requiredQuorumForBallotType(ballotType);
+   ballots[ballotID] = Ballot( ballotID, true, ballotType, ballotName, address1, number1, string1, string2, ballotMinimumEndTime, requiredQuorum );
    openBallotsByName[ballotName] = ballotID;
    _allOpenBallots.add( ballotID );
    
    // Remember that the user made a proposal
    _userHasActiveProposal[msg.sender] = true;
    _usersThatProposedBallots[ballotID] = msg.sender;
    
    emit ProposalCreated(ballotID, ballotType, ballotName);
  }
  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 ))
-   if ( totalVotesCastForBallot(ballotID) < ballot.requiredQuorum)
      return false;
    
    return true;
  }

Assessed type

Error