BriVault

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

Phantom Winner Shares Let Attackers Lock Payouts for Honest Users

Root + Impact

Description

joinEvent records the caller’s current share balance and appends their address to usersAddress every time it is invoked (src/briVault.sol:260-264). cancelParticipation later zeroes stakedAsset but never removes the address or clears userSharesToCountry (src/briVault.sol:280-288). When the owner finalizes, _getWinnerShares iterates the entire array and adds all recorded shares (src/briVault.sol:191-197), including stale entries from users who already exited.

function joinEvent(uint256 countryId) public {
...
uint256 participantShares = balanceOf(msg.sender);
@> userSharesToCountry[msg.sender][countryId] = participantShares;
@> usersAddress.push(msg.sender);
}
function cancelParticipation() public {
...
stakedAsset[msg.sender] = 0;
uint256 shares = balanceOf(msg.sender);
@> _burn(msg.sender, shares); // mapping entry untouched
}
function _getWinnerShares() internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i) {
@> totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
}

Risk

Likelihood: High – the attack uses only public functions and costs at most the participation fee.

Impact:

  • Honest winners receive only pennies because the payout denominator explodes.

  • Vault retains most assets indefinitely, enabling griefing for minimal cost.

Proof of Concept

  1. Attacker deposits once and repeatedly calls joinEvent(winningTeam) before the event starts.

  2. Attacker calls cancelParticipation() to recover their stake (minus fee).

  3. Honest bettors deposit and join the same team.

  4. Owner finalizes with setWinner(winningTeam).

  5. Honest winners call withdraw() and receive a trivial amount; vault balance remains high.
    (Reproduced in test/BriVaultAttack.t.sol:112 via testPhantomSharesLockVault.)

function testPhantomSharesLockVault() public {
vm.startPrank(attacker);
briVault.deposit(FIRST_DEPOSIT, attacker);
for (uint256 i = 0; i < 10; ++i) {
briVault.joinEvent(2);
}
briVault.cancelParticipation(); // leaves phantom entries
vm.stopPrank();
vm.prank(owner);
briVault.setWinner(2);
vm.prank(victimOne);
briVault.withdraw(); // pays only a tiny fraction
}

Recommended Mitigation

  1. Limit each address to a single joinEvent entry or overwrite the prior record instead of appending.

  2. Clear usersAddress entries and userSharesToCountry values when participants cancel or exit.

  3. Recompute winner allocations from live balances during withdrawal, or maintain accurate per-team registries.

Proposed patch (Solidity-like pseudocode):

function joinEvent(uint256 countryId) public {
- userSharesToCountry[msg.sender][countryId] = participantShares;
- usersAddress.push(msg.sender);
+ require(!_hasJoined[msg.sender], "already joined");
+ userSharesToCountry[msg.sender][countryId] = participantShares;
+ usersAddress.push(msg.sender);
+ _hasJoined[msg.sender] = true;
}
function cancelParticipation() public {
@@
- _burn(msg.sender, shares);
+ _burn(msg.sender, shares);
+ delete userSharesToCountry[msg.sender][countryIdOf(msg.sender)];
+ _removeUser(msg.sender);
+ _hasJoined[msg.sender] = false;
}
function _getWinnerShares() internal view returns (uint256 sum) {
- for (uint256 i = 0; i < usersAddress.length; ++i) {
- sum += userSharesToCountry[usersAddress[i]][winnerCountryId];
- }
+ for (uint256 i = 0; i < usersAddress.length; ++i) {
+ address user = usersAddress[i];
+ sum += balanceOf(user);
+ }
Updates

Appeal created

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

`cancelParticipation` Leaves Stale Winner Data

CancelParticipation burns shares but leaves the address inside usersAddress and keeps userSharesToCountry populated.

Support

FAQs

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

Give us feedback!