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

Users can call joinEvent() multiple times

s3mvl4d

Description

  • Each participant should be able to join exactly once (or update their selection in a controlled way) before eventStartDate. The system should maintain a single, deduplicated participant entry and a consistent mapping of shares → selected country.

  • joinEvent(uint256 countryId) pushes the caller’s address on every call and overwrites userToCountry/userSharesToCountry without any guard. This allows the same user to join repeatedly, bloating usersAddress, inflating numberOfParticipants, and causing double counting during finalization when _getWinnerShares() aggregates from usersAddress. There is no check to prevent re‑joining or changing countries 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(); }
​
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
​
// Records (overwrites) shares for chosen country
userSharesToCountry[msg.sender][countryId] = participantShares;
​
// @> No deduplication; same user is pushed every time
usersAddress.push(msg.sender);
​
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
​

Risk

Likelihood: High

  • Users routinely interact with dApps multiple times during the join window (wallet retries, UI refreshes).

  • A griefer can automate multiple joinEvent() calls before eventStartDate, easily spamming entries.

Impact: High

  • Finalize DoS & payout skew: The duplicate entries inflate usersAddress size and overcount totalWinnerShares in _getWinnerShares(), reducing each winner’s payout and potentially pushing setWinner() toward out‑of‑gas in high‑spam scenarios.

  • Misleading analytics/state: numberOfParticipants becomes inaccurate; totalParticipantShares can be inflated by repeated joins even if the user’s actual share balance did not change.

Proof of Concept

  • A single participant can join multiple times, creating duplicates and skewing finalization.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test} from "forge-std/Test.sol";
import {BriVault} from "../src/briVault.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {MockERC20} from "./MockErc20.t.sol";
​
contract MultipleJoinTest is Test {
BriVault vault;
MockERC20 token;
​
address owner = makeAddr("owner");
address user = makeAddr("user");
address fee = makeAddr("fee");
​
uint256 start;
uint256 end;
​
string[48] countries;
​
function setUp() public {
start = block.timestamp + 2 days;
end = start + 30 days;
​
token = new MockERC20("Mock", "M");
token.mint(user, 20 ether);
​
vm.startPrank(owner);
vault = new BriVault(
IERC20(address(token)),
150, // 1.5% fee
start,
fee,
0.0002 ether,
end
);
countries[10] = "Japan";
countries[20] = "Spain";
vault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user);
token.approve(address(vault), type(uint256).max);
​
// One deposit to be eligible
vault.deposit(5 ether, user);
}
​
function test_UserCanJoinMultipleTimes_InflatesState() public {
// First join
vault.joinEvent(10);
// Re-join with the same country
vault.joinEvent(10);
// Re-join with a different country
vault.joinEvent(20);
​
vm.stopPrank();
​
// usersAddress contains the same user 3 times
// (we can't read the array directly if it's internal; if it's public, index reads will show duplicates)
// Effects observable during finalize:
vm.warp(end + 1);
vm.startPrank(owner);
// Expect incorrect winner-share totals due to duplicates when calling setWinner
// (In practice, this can overcount and/or approach OOG depending on spam)
vault.setWinner(10);
vm.stopPrank();
}
}

Recommended Mitigation

  • Prevent multiple joins and keep participant list unique.

​
@@
- address[] public usersAddress;
+ address[] public usersAddress;
+ mapping(address => bool) internal hasJoined;
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) { revert noDeposit(); }
if (countryId >= teams.length) { revert invalidCountry(); }
if (block.timestamp > eventStartDate) { revert eventStarted(); }
+ if (hasJoined[msg.sender]) { revert alreadyJoined(); } // add a custom error
​
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
- usersAddress.push(msg.sender);
- numberOfParticipants++;
- totalParticipantShares += participantShares;
+ // Deduplicate participant list
+ usersAddress.push(msg.sender);
+ hasJoined[msg.sender] = true;
+ numberOfParticipants += 1;
+ totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
Updates

Appeal created

bube Lead Judge 21 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!