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

usersAddress array grows indefinitely

jericho

Root + Impact

Description

  • The _getWinnerShares() function iterates through all addresses in the usersAddress array to calculate the total shares for the winning country. However, the usersAddress array grows indefinitely as users join the event, and the same user can be added multiple times through repeated calls to joinEvent(). This creates a Denial-of-Service vulnerability where the gas cost of setting the winner becomes prohibitively expensive.

// Root cause in the codebase with @> marks to highlight the relevant section
function joinEvent(uint256 countryId) public {
// ... validation logic ...
usersAddress.push(msg.sender); // @> Users can be added multiple times
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}

Risk

Likelihood:

  • The usersAddress array grows with every call to joinEvent() without any duplication checks

  • The same user can be added multiple times by repeatedly calling the function

  • An attacker can spam the contract with many addresses to make the array extremely large

  • The owner must call setWinner() after the event ends, making this vulnerability unavoidable


Impact:

  • Permanent denial of service - once the array becomes too large, setWinner() will exceed block gas limits and become uncallable

  • Locked funds - users cannot withdraw their assets if the winner cannot be set

  • Bricked contract - the entire vault becomes unusable as the core functionality is disabled

Proof of Concept

Join event with same address multiple times,setWinner() becomes too expensive to call when array is large

function test_dos() public {
vm.prank(owner);
briVault.setCountry(countries);
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
// Join event with same address multiple times
for (uint256 i = 0; i < 40000; i++) {
briVault.joinEvent(2);
}
vm.stopPrank();
​
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(2);
}

Recommended Mitigation

Replace the looping calculation with a mapping that tracks total shares per country:

diff --git a/src/briVault.sol b/src/briVault.sol
index bf96d73..e1a240a 100644
--- a/src/briVault.sol
+++ b/src/briVault.sol
@@ -52,7 +52,6 @@ contract BriVault is ERC4626, Ownable {
// Array of teams
string[48] public teams;
- address[] public usersAddress;
// Error Logs
error eventStarted();
@@ -66,6 +65,7 @@ contract BriVault is ERC4626, Ownable {
error eventNotStarted();
error WinnerAlreadySet();
error limiteExceede();
+ error userAlreadyJoined();
event deposited (address indexed _depositor, uint256 _value);
event CountriesSet(string[48] country);
@@ -75,7 +75,7 @@ contract BriVault is ERC4626, Ownable {
mapping (address => uint256) public stakedAsset;
mapping (address => string) public userToCountry;
- mapping(address => mapping(uint256 => uint256)) public userSharesToCountry;
+ mapping(uint256 => uint256) public totalSharesByCountry;
constructor (IERC20 _asset, uint256 _participationFeeBsp, uint256 _eventStartDate, address _participationFeeAddress, uint256 _minimumAmount, uint256 _eventEndDate) ERC4626 (_asset) ERC20("BriTechLabs", "BTT") Ownable(msg.sender) {
@@ -129,7 +129,7 @@ contract BriVault is ERC4626, Ownable {
_setWinner = true;
- _getWinnerShares();
+ totalWinnerShares = totalSharesByCountry[winnerCountryId];
_setFinallizedVaultBalance();
@@ -185,18 +188,6 @@ contract BriVault is ERC4626, Ownable {
return teams[countryId];
}
- /**
- @notice get winnerShares
- */
- function _getWinnerShares () internal returns (uint256) {
-
- for (uint256 i = 0; i < usersAddress.length; ++i){
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
- return totalWinnerShares;
- }
-
function _getParticipationFee(uint256 assets) internal view returns (uint256) {
return (assets * participationFeeBsp) / BASE;
}
@@ -253,14 +244,13 @@ contract BriVault is ERC4626, Ownable {
revert eventStarted();
}
-
+ if (bytes(userToCountry[msg.sender]).length != 0) {
+ revert userAlreadyJoined();
+ }
userToCountry[msg.sender] = teams[countryId];
-
uint256 participantShares = balanceOf(msg.sender);
- userSharesToCountry[msg.sender][countryId] = participantShares;
-
- usersAddress.push(msg.sender);
+ totalSharesByCountry[countryId] += participantShares;
numberOfParticipants++;
totalParticipantShares += participantShares;
Updates

Appeal created

bube Lead Judge 20 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.

Support

FAQs

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

Give us feedback!