Root + Impact
Description
Root Cause
The contract allows users to deposit additional tokens after calling joinEvent(). However, joinEvent() takes a snapshot of shares at that moment and stores it in userSharesToCountry, which never gets updated if the user deposits more tokens.
Vulnerable code:
function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
_mint(msg.sender, participantShares);
}
function joinEvent(uint256 countryId) public {
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
totalParticipantShares += participantShares;
}
Risk
Likelihood:
Risk (Likelihood + Impact):
Likelihood:
Users will deposit after joining because there is no validation preventing it. The protocol rules imply a single deposit-then-join flow, but nothing enforces this constraint technically.
Any user can call deposit() multiple times before the event starts, and nothing stops them from calling it after joinEvent().
Impact:
This creates state inconsistency between actual shares and recorded shares:
Scenario 1: Undercounted shares
Alice deposits 500 tokens → receives 500 shares
Alice calls joinEvent(3) → userSharesToCountry[alice][3] = 500
Alice deposits another 500 tokens → now has 1000 total shares
userSharesToCountry[alice][3] still shows only 500 shares
totalParticipantShares only counted the initial 500
Scenario 2: Winner calculation errors
When _getWinnerShares() runs, it uses userSharesToCountry values
These values are stale and don't reflect additional deposits
totalWinnerShares is calculated incorrectly
All winners receive wrong proportions
Scenario 3: State corruption
balanceOf(user) shows 1000 shares
userSharesToCountry[user][country] shows 500 shares
totalParticipantShares doesn't match sum of balanceOf() for all users
System state becomes internally inconsistent
This violates accounting integrity and can lead to incorrect fund distributions.
Impact:
Proof of Concept
---
### **Proof of Concept:**
```solidity
function testDepositAfterJoinCreatesInconsistency() public {
uint256 firstDeposit = 500e18;
uint256 secondDeposit = 500e18;
vm.startPrank(alice);
token.approve(address(vault), firstDeposit + secondDeposit);
vault.deposit(firstDeposit, alice);
uint256 sharesAfterFirst = vault.balanceOf(alice);
vault.joinEvent(5);
uint256 recordedShares = vault.userSharesToCountry(alice, 5);
assertEq(recordedShares, sharesAfterFirst);
vault.deposit(secondDeposit, alice);
uint256 totalSharesNow = vault.balanceOf(alice);
vm.stopPrank();
assertGt(totalSharesNow, recordedShares);
uint256 stillRecorded = vault.userSharesToCountry(alice, 5);
assertEq(stillRecorded, recordedShares);
console.log("Actual shares:", totalSharesNow);
console.log("Recorded shares:", stillRecorded);
console.log("Untracked shares:", totalSharesNow - stillRecorded);
}
function testWinnerSharesUseStaleData() public {
vm.startPrank(alice);
token.approve(address(vault), 1000e18);
vault.deposit(500e18, alice);
vault.joinEvent(0);
vault.deposit(500e18, alice);
vm.stopPrank();
vm.startPrank(bob);
token.approve(address(vault), 500e18);
vault.deposit(500e18, bob);
vault.joinEvent(0);
vm.stopPrank();
vm.warp(vault.eventEndDate() + 1);
vm.prank(owner);
vault.setWinner(0);
uint256 totalWinner = vault.totalWinnerShares();
uint256 aliceActual = vault.balanceOf(alice);
uint256 bobActual = vault.balanceOf(bob);
assertLt(totalWinner, aliceActual + bobActual);
}
```
Expected: Shares tracked accurately reflect all deposits
Actual: Shares tracked at join time, never updated
Recommended Mitigation
- remove this code
+ add this code
- Users can deposit multiple times after joining
- userSharesToCountry stores stale snapshot values
+ Enforce single deposit before join OR use actual balances
Option 1: Prevent deposits after joining
mapping(address => bool) public hasJoined;
function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
+ require(!hasJoined[msg.sender], "Cannot deposit after joining");
// ... rest of function
}
function joinEvent(uint256 countryId) public {
// ... validation ...
+ hasJoined[msg.sender] = true;
// ... rest of function
}
Option 2: Always use actual balanceOf() instead of cached values
function _getWinnerShares() internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
+ if (keccak256(abi.encodePacked(userToCountry[user])) ==
+ keccak256(abi.encodePacked(winner))) {
+ totalWinnerShares += balanceOf(user); // Use actual balance
+ }
}
return totalWinnerShares;
}
This ensures calculations always use current share balances, not stale snapshots.
