Submission Details
Severity:
Multiple joinEvent() Calls Create Duplicate Entries
Multiple joinEvent() Calls Create Duplicate Entries
Description
The normal behavior is that each user should only be able to join the tournament once by selecting a single team to bet on, and should be added to the participants array exactly once.
The issue is that
joinEvent()has no check to prevent multiple calls by the same user, allowing them to call it repeatedly which adds their address tousersAddressarray multiple times, incrementsnumberOfParticipantsfor each call, and creates ghost entries that waste gas and corrupt accounting calculations.
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
// @> No check if user already joined!
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
usersAddress.push(msg.sender); // @> User added again!
numberOfParticipants++; // @> Counter incremented again!
totalParticipantShares += participantShares; // @> Shares double-counted!
emit joinedEvent(msg.sender, countryId);
}
Risk
Likelihood:
Will occur whenever users accidentally double-click, retry failed transactions, or intentionally call function multiple times
No technical skill required - simple repeat function call
Likely to happen frequently in production due to UI issues or user error
No cost barrier - users already deposited so calling joinEvent again costs minimal gas
Impact:
numberOfParticipantsbecomes inflated (incorrect but not directly damaging)Gas waste when looping through
usersAddressarray with duplicatesIn
_getWinnerShares(), same user counted multiple times causing incorrecttotalWinnerSharesIncorrect
totalWinnerSharesleads to wrong withdrawal amounts for all winnersDoes not directly steal funds but creates unfair distribution
Proof of Concept
// SPDX-License-Identifier: MIT
import "./BriVault.sol";
contract MultiJoinExploit {
BriVault public vault;
function demonstrateMultipleJoins() external {
// Initial state: User deposited 1000 USDC, has 1000 shares
// Join #1
vault.joinEvent(0); // Brazil
// State:
// userToCountry[this] = "Brazil"
// userSharesToCountry[this][0] = 1000
// usersAddress = [this]
// numberOfParticipants = 1
// totalParticipantShares = 1000
// Join #2 (same team)
vault.joinEvent(0);
// State:
// userToCountry[this] = "Brazil" (same)
// userSharesToCountry[this][0] = 1000 (overwritten)
// usersAddress = [this, this] ← DUPLICATE
// numberOfParticipants = 2 ← WRONG
// totalParticipantShares = 2000 ← WRONG
// Join #3
vault.joinEvent(0);
// State:
// usersAddress = [this, this, this]
// numberOfParticipants = 3
// totalParticipantShares = 3000
}
function demonstrateWinnerCalculationImpact() external {
// Assume this is the scenario from above
// User appears 3 times in usersAddress
// User has 1000 shares
// When setWinner(0) is called (Brazil wins):
// _getWinnerShares() executes:
// totalWinnerShares = 0;
// for (i = 0; i < 3; i++) {
// user = usersAddress[i]; // this, this, this
// totalWinnerShares += userSharesToCountry[user][0];
// }
// Iteration 1: totalWinnerShares += 1000 = 1000
// Iteration 2: totalWinnerShares += 1000 = 2000
// Iteration 3: totalWinnerShares += 1000 = 3000
// totalWinnerShares = 3000 (should be 1000!)
}
function demonstrateWithdrawalLoss() external {
// Continuing from above:
// - Vault has 3000 USDC total
// - This user is ONLY winner
// - User has 1000 shares
// - totalWinnerShares = 3000 (incorrectly counted 3x)
// User withdraws:
// assetToWithdraw = mulDiv(1000, 3000, 3000)
// assetToWithdraw = 1000 USDC
// User receives: 1000 USDC
// User SHOULD receive: 3000 USDC (100% of vault)
// Loss: 2000 USDC locked forever in contract
}
}
// Multi-user scenario showing the accounting corruption:
contract MultiUserScenario {
/*
Scenario:
- Alice deposits 1000 USDC, joins Brazil once
- Bob deposits 2000 USDC, joins Brazil 5 times (by accident)
- Carol deposits 1500 USDC, joins Brazil once
Total deposits: 4500 USDC
Total shares: 4500
When Brazil wins:
_getWinnerShares() loop:
- i=0: user=Alice, totalWinnerShares += 1000 = 1000
- i=1: user=Bob, totalWinnerShares += 2000 = 3000
- i=2: user=Bob, totalWinnerShares += 2000 = 5000
- i=3: user=Bob, totalWinnerShares += 2000 = 7000
- i=4: user=Bob, totalWinnerShares += 2000 = 9000
- i=5: user=Bob, totalWinnerShares += 2000 = 11000
- i=6: user=Carol, totalWinnerShares += 1500 = 12500
totalWinnerShares = 12500 (should be 4500!)
Withdrawals:
- Alice: (1000 * 4500) / 12500 = 360 USDC (should get 1000)
- Bob: (2000 * 4500) / 12500 = 720 USDC (should get 2000)
- Carol: (1500 * 4500) / 12500 = 540 USDC (should get 1500)
Total withdrawn: 1620 USDC
Locked in vault: 2880 USDC
Everyone loses money due to Bob's duplicate joins!
*/
}
// Gas waste demonstration:
contract GasWasteDemo {
function calculateGasWaste() external pure returns (uint256) {
// If 100 users each join 3 times:
// usersAddress.length = 300 (should be 100)
// In setWinner(), _getWinnerShares() loops 300 times
// Each iteration: ~5000 gas (SLOAD + SLOAD + addition)
// Total: 1,500,000 gas wasted (should be 500,000)
// Extra cost: 1,000,000 gas
// At 50 gwei: 0.05 ETH wasted
// At $3000 ETH: $150 wasted on unnecessary computation
return 1_000_000; // Wasted gas
}
}
Recommended Mitigation
contract BriVault is ERC4626, Ownable {
+ mapping(address => bool) public hasJoinedEvent;
+ error AlreadyJoined();
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
+ if (hasJoinedEvent[msg.sender]) {
+ revert AlreadyJoined();
+ }
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);
+ hasJoinedEvent[msg.sender] = true;
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}
function cancelParticipation() public {
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
+ hasJoinedEvent[msg.sender] = false;
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
}
Updates
Appeal created
bube 21 days ago
Submission Judgement Published Assigned finding tags:
Duplicate registration through `joinEvent`
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.