Summary
If the Organizer calls the withdrawPredictionFees function anytime before unapproved users could cancelRegistration , then entranceFees meant for those users will be drained.
Vulnerability Details
According to the design of the Dapp,
The funds collected from this prediction fees are used to cover the costs of the hall and the Organizer must be able to withdraw those funds at any time.
Only the predictionFees should be withdrawn by organizer ANYTIME he wants; But the withdrawPredictionFees function is withdrawing ALL funds including entranceFee meant for unapproved users who may not have canceled their registration as at when organizer is calling the withdraw function;
If Organizer keeps withdrawing every now and then and unapproved users call cancelRegistration later on, this may lead to depletion of the reward pool(30 players entranceFee). Hence reward distribution problems
PoC (Proof of Code)
pragma solidity ^0.8.13;
import {Test, console} from "forge-std/Test.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
import {ThePredicter} from "../src/ThePredicter.sol";
import {ScoreBoard} from "../src/ScoreBoard.sol";
contract ThePredicterTest is Test {
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
address public organizer = makeAddr("Ivan");
function setUp() public {
vm.startPrank(organizer);
scoreBoard = new ScoreBoard();
thePredicter = new ThePredicter(
address(scoreBoard),
0.04 ether,
0.0001 ether
);
scoreBoard.setThePredicter(address(thePredicter));
vm.stopPrank();
}
function test_organizerWithdrawNonApprovedUsersFees() public{
for (uint256 i = 0; i < 20; ++i) {
address not_approved = makeAddr(string.concat("no-approved-user", Strings.toString(i)));
vm.startPrank(not_approved);
vm.deal(not_approved, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
}
uint256 unapprovedUsersFunds = address(thePredicter).balance;
for (uint256 i = 0; i < 30; ++i) {
address user = makeAddr(string.concat("user", Strings.toString(i)));
vm.startPrank(user);
vm.deal(user, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(user);
vm.stopPrank();
}
uint256 vaultBal = address(thePredicter).balance;
vm.prank(organizer);
thePredicter.withdrawPredictionFees();
uint256 approvePlayersFunds = address(thePredicter).balance;
assertEq(vaultBal, 2 ether);
assertLe(vaultBal - approvePlayersFunds, 1.2 ether);
assertEq(organizer.balance, unapprovedUsersFunds);
}
}
Impact
Tools Used
Manual review
foundry test
Recommendations
In the thePredicter.sol contract do the following;
// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
import {Address} from "@openzeppelin/contracts/utils/Address.sol";
import {ScoreBoard} from "./ScoreBoard.sol";
import {console} from "forge-std/Test.sol";
contract ThePredicter {
+ uint256 registeredUsersCount;
function register() public payable {
if (msg.value != entranceFee) {
revert ThePredicter__IncorrectEntranceFee();
}
if (block.timestamp > START_TIME - 14400) {
revert ThePredicter__RegistrationIsOver();
}
if (playersStatus[msg.sender] == Status.Pending) {
revert ThePredicter__CannotParticipateTwice();
}
+ registeredUsersCount++;
playersStatus[msg.sender] = Status.Pending;
}
function cancelRegistration() public {
if (playersStatus[msg.sender] == Status.Pending) {
+ registeredUsersCount--;
(bool success, ) = msg.sender.call{value: entranceFee}("");
require(success, "Failed to withdraw");
playersStatus[msg.sender] = Status.Canceled;
return;
}
revert ThePredicter__NotEligibleForWithdraw();//@audit
}
function withdrawPredictionFees() public {
if (msg.sender != organizer) {
revert ThePredicter__NotEligibleForWithdraw();//@audit wrong throw==
}
- uint256 fees = address(this).balance - players.length * entranceFee;
+ uint256 fees = address(this).balance - registeredUsersCount * entranceFee;
(bool success, ) = msg.sender.call{value: fees}("");
require(success, "Failed to withdraw");
}
}
