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

BriVault:joinEvent function allows multiple calls from the same address, potentially causing a DoS due to unbounded growth of the BriVault:usersAddress array

eliab256

Root + Impact

Description

  • Once a user has called BriVault:deposit and has assets staked, they can call BriVault:joinEvent to place their bet.
    However, after the bet is placed, the user should not be able to call BriVault:joinEvent again unless they first cancel their previous participation.

  • The issue lies in the fact that BriVault:joinEvent does not check whether a user is already registered, allowing multiple registrations with a single deposit. A malicious actor could exploit this by repeatedly calling BriVault:joinEvent thousands of times, causing the BriVault:usersAddress array to grow excessively large and potentially leading to a Denial of Service (DoS) when iterating over it in a for loop.

function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
// Ensure countryId is a valid index in the `teams` array
if (countryId >= teams.length) {
revert invalidCountry();
}
​
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
@> //does not check if msg.sender is already registered
​
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);
}

Risk

Likelihood:

  • Whenever a malicious user wants to break the contract and holds at least BriVault:minimumAmountToJoin plus the required fee.

Impact:

  • Permanent DoS: When the owner attempts to call BriVault:setWinner, the transaction will always revert because the for loop inside the internal function BriVault:_getWinnerShares will run out of gas. As a result, the contract will become unusable, and all the underlying tokens will remain permanently locked.

Proof of Concept

In the test, an account named attacker performs a deposit on BriVault:deposit and then calls BriVault:joinEvent 35,000 times, causing the BriVault:usersAddress array to grow excessively large. As a result, when the owner later calls BriVault:setWinner, the call reverts due to running out of gas while iterating the array inside _getWinnerShares, producing a permanent DoS and leaving the underlying tokens locked.

The setup is the same as the tests performed by the developer.

function testDOSAttackOnUsersAddressArray() public {
uint256 depositAmount = 1 ether;
vm.startPrank(attacker);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, attacker);
vm.stopPrank();
for(uint256 i = 0; i < 35000; i++) {
vm.prank(attacker);
briVault.joinEvent(0);
}
​
vm.warp(briVault.eventEndDate() +1);
vm.startPrank(owner);
uint256 gasLimit = 30_000_000;
uint256 gasBefore = gasleft();
briVault.setWinner(0);
uint256 gasUsed = gasBefore - gasleft();
vm.stopPrank();
assertGt(gasLimit, gasUsed, "OutOfGas");
​
}

Recommended Mitigation

To mitigate this issue, one possible (but inefficient) approach would be to iterate through the BriVault:usersAddress array on each call to verify whether the address has already joined. However, this solution would be too gas-intensive.

A more efficient approach is to introduce a mapping (for example, mapping(address => bool)) that tracks whether an address has already joined the event. This mapping should be updated whenever a user successfully calls BriVault:joinEvent and reset if the user withdraws or cancels their participation.

Additionally, a custom error (e.g., AlreadyJoined) should be defined and triggered in case the same address attempts to join again.

This solution prevents multiple registrations from the same address while keeping gas consumption minimal.

The BriVault:cancelParticipation function also requires a modification, since once an attacker cancels their participation, they receive the refund amount (minus the participation fee), but their address is not removed from the usersAddress array.


NOTE: It is still theoretically possible to attempt a Denial of Service (DoS) attack by repeatedly joining the event with multiple addresses. However, since each new BriVault:joinEvent call now requires a unique account and a separate deposit, the cost of performing such an attack becomes prohibitively high, making it economically infeasible in practice.

+ mapping (address => bool) hasJoined;
​
+ error alreadyJoined();
+ error notJoined();
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
+ if(hasJoined[msg.sender]){
+ revert alreadyJoined();
+ }
​
if (countryId >= teams.length) {
revert invalidCountry();
}
​
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
​
+ hasJoined[msg.sender] = true;
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);
}
​
function cancelParticipation() public {
if (block.timestamp >= eventStartDate) revert eventStarted();
+ if(!hasJoined[msg.sender]){
+ revert notJoined();
+ }
​
uint256 refundAmount = stakedAsset[msg.sender];
require(refundAmount > 0, "Nothing to refund");
​
stakedAsset[msg.sender] = 0;
+ hasJoined[msg.sender] = false;
+ for (uint256 i = 0; i < usersAddress.length; i++) {
+ if (usersAddress[i] == msg.sender) {
+ usersAddress[i] = usersAddress[usersAddress.length - 1];
+ usersAddress.pop();
+ break;
+ }
+ }
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Duplicate registration through `joinEvent`

Support

FAQs

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

Give us feedback!