Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Invalid

deploying multiple contracts with `_distributeRewards` an extensive list of voters, leading to increased gas consumption during the reward distribution process

Summary

The contract is susceptible to a gas exhaustion vulnerability due to the potential for a large loop iteration in the _distributeRewards function, where rewards are distributed to voters. This could be exploited by an attacker deploying multiple contracts with an extensive list of voters, leading to increased gas consumption during the reward distribution process.

Vulnerability Details

function _distributeRewards() private {
// get number of voters for & against
uint256 totalVotesFor = s_votersFor.length;
uint256 totalVotesAgainst = s_votersAgainst.length;
uint256 totalVotes = totalVotesFor + totalVotesAgainst;
// rewards to distribute or refund. This is guaranteed to be
// greater or equal to the minimum funding amount by a check
// in the constructor, and there is intentionally by design
// no way to decrease or increase this amount. Any findings
// related to not being able to increase/decrease the total
// reward amount are invalid
uint256 totalRewards = address(this).balance;
// if the proposal was defeated refund reward back to the creator
// for the proposal to be successful it must have had more `For` votes
// than `Against` votes
if (totalVotesAgainst >= totalVotesFor) {
// proposal creator is trusted to create a proposal from an address
// that can receive ETH. See comment before declaration of `s_creator`
_sendEth(s_creator, totalRewards);
}
// otherwise the proposal passed so distribute rewards to the `For` voters
else {
uint256 rewardPerVoter = totalRewards / totalVotes;
for (uint256 i; i < totalVotesFor; ++i) {
// proposal creator is trusted when creating allowed list of voters,
// findings related to gas griefing attacks or sending eth
// to an address reverting thereby stopping the reward payouts are
// invalid. Yes pull is to be preferred to push but this
// has not been implemented in this simplified version to
// reduce complexity & help you focus on finding the
// harder to find bug
// if at the last voter round up to avoid leaving dust; this means that
// the last voter can get 1 wei more than the rest - this is not
// a valid finding, it is simply how we deal with imperfect division
if (i == totalVotesFor - 1) {
rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
}
_sendEth(s_votersFor[i], rewardPerVoter);
}
}
}

Impact

An attacker could intentionally create a large number of voters, causing the loop in the _distributeRewards function to iterate a significant number of times. As a result, the contract may run out of gas, leading to a failed transaction or a denial of service (DoS) attack.

Tools Used

Manual

Recommendations

Consider using a withdrawal pattern to mitigate the gas exhaustion vulnerability. Instead of distributing rewards in a single large loop, allow voters to claim their rewards individually. Like here, modification to the _distributeRewards function:

// Updated function to distribute rewards using a withdrawal pattern
function _distributeRewards() private {
// ...
// if the proposal passed, distribute rewards to the `For` voters
else {
for (uint256 i; i < totalVotesFor; ++i) {
// ...
_sendEth(s_votersFor[i], rewardPerVoter);
}
}
}
// New function to allow voters to claim their rewards individually
function claimReward() external {
require(s_voters[msg.sender] == VOTED, "You haven't voted or already claimed your reward");
require(!s_votingComplete, "Voting has already been completed");
// Update voter state
s_voters[msg.sender] = DISALLOWED;
// Distribute individual reward
_sendEth(msg.sender, rewardPerVoter);
}
Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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