`BootstrapBallot#vote()` doesn't check if the ballot has been completed or not

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/launch/BootstrapBallot.sol#L48-L65

Vulnerability details

Impact

Users can vote and get airdrop qualification even after the completion of the ballot, as long as the ballot has not been finalized.

Proof of Concept

An eligible user can vote on whether or not to start up the exchange by calling BootstrapBallot#vote(), then the voter will be authorized to receive the airdrop.

function vote( bool voteStartExchangeYes, bytes calldata signature ) external nonReentrant
{
  require( ! hasVoted[msg.sender], "User already voted" );

  // Verify the signature to confirm the user is authorized to vote
  bytes32 messageHash = keccak256(abi.encodePacked(block.chainid, msg.sender));
  require(SigningTools._verifySignature(messageHash, signature), "Incorrect BootstrapBallot.vote signatory" );

  if ( voteStartExchangeYes )
    startExchangeYes++;
  else
    startExchangeNo++;

  hasVoted[msg.sender] = true;

  // As the whitelisted user has retweeted the launch message and voted, they are authorized to the receive the airdrop.
  airdrop.authorizeWallet(msg.sender);
}

Once the ballot completed, any one can call BootstrapBallot#finalizeBallot() to finalize the ballot. The exchange will be actived if YES votes is greater than NO votes.

  function finalizeBallot() external nonReentrant
  {
    require( ! ballotFinalized, "Ballot has already been finalized" );
    require( block.timestamp >= completionTimestamp, "Ballot is not yet complete" );

    if ( startExchangeYes > startExchangeNo )
    {
      exchangeConfig.initialDistribution().distributionApproved();
      exchangeConfig.dao().pools().startExchangeApproved();

      startExchangeApproved = true;
    }

    emit BallotFinalized(startExchangeApproved);

    ballotFinalized = true;
  }

However, a voter can still cast their vote even after the completion of the ballot, as long as the ballot has not been finalized. Besides the voter is authorized to receive the airdrop, the voting result might be altered due to the inclusion of new votes.

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

  function test_voteAfterCompletion() public {
    // Voting stage (yesVotes: 2, noVotes: 0)
    assertEq(airdrop.isAuthorized(alice), false);
    bytes memory sig = abi.encodePacked(aliceVotingSignature);
    vm.prank(alice);
    bootstrapBallot.vote(true, sig);
    assertEq(airdrop.isAuthorized(alice), true);

    // Increase current blocktime to be greater than completionTimestamp
    vm.warp( bootstrapBallot.completionTimestamp());
    //@audit-info bob is not qualified for airdrop
    assertEq(airdrop.isAuthorized(bob), false);
    //@audit-info current time is greater than completionTimestamp
    assertTrue(block.timestamp >= bootstrapBallot.completionTimestamp());
    //@audit-info exchange is eligible to be actived
    assertTrue(bootstrapBallot.startExchangeYes() > bootstrapBallot.startExchangeNo());
    //@audit-info bob casts vote successfully
    sig = abi.encodePacked(bobVotingSignature);
    vm.prank(bob);
    bootstrapBallot.vote(false, sig);
    //@audit-info bob is qualified for airdrop now
    assertEq(airdrop.isAuthorized(bob), true);
    //@audit-info the voting result is flipped. exchange is not eligible to be actived
    assertTrue(bootstrapBallot.startExchangeYes() <= bootstrapBallot.startExchangeNo());
  }

Tools Used

Manual review

Recommended Mitigation Steps

Make sure no one can cast vote after completionTimestamp:

function vote( bool voteStartExchangeYes, bytes calldata signature ) external nonReentrant
{
  require( ! hasVoted[msg.sender], "User already voted" );

  // Verify the signature to confirm the user is authorized to vote
  bytes32 messageHash = keccak256(abi.encodePacked(block.chainid, msg.sender));
  require(SigningTools._verifySignature(messageHash, signature), "Incorrect BootstrapBallot.vote signatory" );
+ require( block.timestamp < completionTimestamp, "Ballot is complete" );

  if ( voteStartExchangeYes )
    startExchangeYes++;
  else
    startExchangeNo++;

  hasVoted[msg.sender] = true;

  // As the whitelisted user has retweeted the launch message and voted, they are authorized to the receive the airdrop.
  airdrop.authorizeWallet(msg.sender);
}

Assessed type

Error

[c4-judge]

Picodes marked the issue as duplicate of #100

[c4-judge]

Picodes marked the issue as selected for report

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

Downgrading to Low as it may be a design choice: the ballot has passed the completion timestamp so can be finalized but has not been finalized yet so voting is still open

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

Voting reverts after the ballot has been finalized.

The authorizeWallet call in vote requires that claiming is not allowed (which it is on ballot finalization): require( ! claimingAllowed, "Cannot authorize after claiming is allowed" );

[c4-judge]

Picodes marked the issue as not selected for report

[c4-judge]

Picodes marked the issue as grade-b