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

Missing Initialization of `s_creator` in Constructor (Critical Loss of Funds)

imth3ak

Summary

The constructor of the contract does not initialize the s_creator variable, which is supposed to store the address of the proposal creator. This leaves the s_creator variable with the default value of zero address. This could allow anyone to call the refund function and drain the contract balance, as the refund function does not check that msg.sender == s_creator.

Vulnerability Details

The constructor of the contract is responsible for setting the initial state of the contract, such as the allowed voters, the reward amount, and the proposal creator. However, the constructor does not initialize the s_creator variable, which is supposed to store the address of the proposal creator. This leaves the s_creator variable with the default value of zero address, which is a special address that can receive funds but cannot send them.

This could allow anyone to call the refund function and drain the contract balance, as the refund function does not check that msg.sender == s_creator. The refund function is supposed to refund the reward amount back to the proposal creator if the proposal is defeated or the voting is not completed within the deadline. However, since the s_creator variable is zero address, anyone can call the refund function and claim the reward amount as their own. This could result in a loss of funds for the proposal creator and the voters.

Impact

The impact of this vulnerability is that the contract could lose all the funds that it holds as a reward for the voters. Anyone could exploit this vulnerability by calling the refund function and transferring the contract balance to their own address. 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 anyone could exploit this vulnerability:

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

  • Call the refund function from any address that is not the proposal creator or one of the allowed voters.

  • Observe that the contract balance is transferred to the caller 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;
​
function testRefund() 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);
​
// check the initial contract balance
console.log("Initial contract balance: %s wei", address(votingBooth).balance);
​
// call the refund function from any address that is not the proposal creator or one of the allowed voters
votingBooth.refund{from: 0x4567890123456789012345678901234567890123}();
​
// check the final contract balance
console.log("Final contract balance: %s wei", address(votingBooth).balance);
​
// check the balance of the caller
console.log("Caller balance: %s wei", 0x4567890123456789012345678901234567890123.balance);
}
}
​

Logs

Initial contract balance: 1000000000000000000 wei
Final contract balance: 0 wei
Caller balance: 1000000000000000000 wei
​

Recommendations

To fix this issue, I recommend that you assign msg.sender to s_creator in the constructor and add a require statement in the refund function to check that msg.sender == s_creator. This way, you can ensure that only the proposal creator can call the refund function and claim the reward amount.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;
​
import "./Math.sol";
​
contract VotingBooth {
// ... omitted for brevity
​
// proposal creator
address public s_creator;
​
// constructor
constructor(address[] memory allowList) payable {
// require that the reward amount is greater than or equal to the minimum funding amount
require(msg.value >= MIN_FUNDING_AMOUNT, "DP: reward amount must be greater than or equal to the minimum funding amount");
​
// require that the allowed list of voters is not empty
require(allowList.length > 0, "DP: allowed list of voters must not be empty");
​
// set the total number of allowed voters
s_totalAllowedVoters = allowList.length;
​
// set the allowed voters in the mapping
for (uint256 i; i < allowList.length; ++i) {
s_voters[allowList[i]] = ALLOWED;
}
​
// set the deadline for voting
s_deadline = block.timestamp + VOTING_PERIOD;
​
// initialize s_creator with msg.sender
s_creator = msg.sender;
}
​
// ... omitted for brevity
​
// refunds the reward amount back to the proposal creator
// if the proposal is defeated or the voting is not completed
// within the deadline
function refund() external {
// require that the caller is the proposal creator
require(msg.sender == s_creator, "DP: caller is not the proposal creator"); // add this check
​
// require that the voting is not active
require(!isActive(), "DP: voting is still active");
​
// require that the proposal is not successful
require(!isSuccessful(), "DP: proposal is successful");
​
// send the reward amount back to the proposal creator
_sendEth(s_creator, address(this).balance);
}
​
// ... 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.