First Flight #6: Voting Booth
First Flight #6
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Auditing Report

RSecurity

Findings

High

[H-1] Invalid reward distribution for the For voters leading to stuck reward in the Smart Contract.

Description: When a vote reaches the quorom, the reward should be distributed amoung the For voters. However, when we take a look over the distribution of the voters, we can see that it does not match the reward amount they should receive. This is due to a mis calculation of the reward by individus. To compute the reward to each individu, the smart contract take the total rewards, which is the balance of the smart contract and divide it by the total numbers of votes. However, we should used the total number of For instead, else the totality of the reward will not be distributed.

uint256 rewardPerVoter = totalRewards / totalVotes;

Impact: The distribution of rewards is not valid, as a proportion is not distributed for the For voters. Additionally, when a vote is terminated, tokens may be lost on the smart contract and no one can retrieve them.

Proof of Concept:

PoC

Place the following test into PuppyRaffleTest.t.sol.

function test_invalidBalance() public {
​
console.log("Start balance:", address(booth).balance);
​
vm.prank(address(0x1));
booth.vote(false);
vm.prank(address(0x2));
booth.vote(true);
​
vm.prank(address(0x3));
booth.vote(true);
​
assert(!booth.isActive() && address(booth).balance > 0);
​
console.log("Address 1:", address(0x01).balance);
console.log("Address 2:", address(0x02).balance);
console.log("Address 3:", address(0x03).balance);
​
console.log("After balance:", address(booth).balance);
}

Recommended Mitigation:

Instead of using the total number of voters, it can be interesting to use the total of For voters:

In line VotingBooth.sol:192, we can change:

-uint256 rewardPerVoter = totalRewards / totalVotes;
+uint256 rewardPerVoter = totalRewards / totalVotesFor;

And we can also change line VotingBooth.sol:207:

-rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
+rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotesFor, Math.Rounding.Ceil);

Also, defined what you intended by reward, is it the totality of the pool, as it is not a defined information, I suspect this could be something that wasn't thought of beforehand.

Low

L-1: Solidity pragma should be specific, not wide

Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of pragma solidity ^0.8.0;, use pragma solidity 0.8.0;

  • Found in src/VotingBooth.sol Line: 2

    pragma solidity ^0.8.23;

L-2: PUSH0 is not supported by all chains

Solc compiler version 0.8.20 switches the default target EVM version to Shanghai, which means that the generated bytecode will include PUSH0 opcodes. Be sure to select the appropriate EVM version in case you intend to deploy on a chain other than mainnet like L2 chains that may not support PUSH0, otherwise deployment of your contracts will fail.

  • Found in src/VotingBooth.sol Line: 2

    pragma solidity ^0.8.23;

Informational

I-1: VotingBooth.DISALLOWED is never used in VotingBooth.

In the contract, the vote has a state called disallowed. However, this state is never used in the code.

uint8 private constant DISALLOWED = 0;

It seems this is a functionnality that is not intended to be use. If it is the case, I think it should be removed or implemented in the code.

I-2: Defined immutable variable

Some of the variables could be defined as immutable as there are only defined in the constructor of the smart contract. We notice the following variables impacted: s_creator and s_totalAllowedVoters. Here is our recommendation:

-address private s_creator;
+address private immutable s_creator;
​
// total number of allowed voters
-uint256 private s_totalAllowedVoters;
+uint256 private immutable s_totalAllowedVoters;
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

VotingBooth._distributeRewards(): Incorrect computation of rewardPerVoter

PUSH0 not supported for solidity solidity ^0.8.20

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.