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

Duplicate Team Names Let Losing Bettors Drain the Pool

galer ah

Root + Impact

Description

withdraw() validates winners by comparing keccak256(abi.encodePacked(userToCountry[msg.sender])) with the stored winner string (src/briVault.sol:257, src/briVault.sol:299-303). If the owner configures two slots in teams with identical (or empty) names, bettors on the losing index still pass the string check. _getWinnerShares only credits the actual winning index, so a losing participant can call withdraw() first and receive the entire prize pool while legitimate winners subsequently revert.

function joinEvent(uint256 countryId) public {
...
@> userToCountry[msg.sender] = teams[countryId];
}
​
function withdraw() external winnerSet {
...
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
@> keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
uint256 assetToWithdraw = Math.mulDiv(shares, finalizedVaultAsset, totalWinnerShares);
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
}

Risk

Likelihood: Medium – it relies on a misconfigured tournament (duplicate or unset team names), but such operator mistakes are common and easy for attackers to notice.

Impact:

  • A losing bettor can withdraw the entire vault balance if their team name matches the declared winner.

  • Honest winners revert on withdraw() after the vault is emptied.

Proof of Concept

  1. Owner sets teams[0] = "Alpha" and teams[1] = "Alpha".

  2. User A joins team 0; User B joins team 1.

  3. After the event, owner calls setWinner(0).

  4. User B (loser) calls withdraw() first and receives the full pot.

  5. User A (winner) calls withdraw() and reverts because the vault balance is zero.
    (Reproduced in test/BriVaultAttack.t.sol:228 via testDuplicateTeamNameLetsLoserStealPool.)

function testDuplicateTeamNameLetsLoserStealPool() public {
string[48] memory dupCountries = countries;
dupCountries[1] = countries[0];
vm.prank(owner);
briVault.setCountry(dupCountries);
​
briVault.deposit(FIRST_DEPOSIT, winner);
briVault.joinEvent(0);
briVault.deposit(FIRST_DEPOSIT, loser);
briVault.joinEvent(1); // identical name
​
vm.prank(owner);
briVault.setWinner(0);
​
vm.prank(loser);
briVault.withdraw(); // drains vault despite losing index
}

Recommended Mitigation

  1. Reject duplicate or empty team identifiers when calling setCountry (e.g., track seen hashes or enforce bytes(countries[i]).length > 0 and uniqueness).

  2. Store and compare canonical country IDs (indices) instead of strings when validating winners and participants.

  3. Add regression tests covering misconfigured tournaments to ensure losers cannot satisfy the winner check.

Proposed patch (Solidity-like pseudocode):

function setCountry(string[48] memory countries) public onlyOwner {
for (uint256 i = 0; i < countries.length; ++i) {
- teams[i] = countries[i];
+ bytes memory name = bytes(countries[i]);
+ require(name.length > 0, "Empty team name");
+ bytes32 hash = keccak256(name);
+ require(!_teamNameUsed[hash], "Duplicate team name");
+ _teamNameUsed[hash] = true;
+ teams[i] = countries[i];
}
}
​
function joinEvent(uint256 countryId) public {
...
- userToCountry[msg.sender] = teams[countryId];
+ userCountryId[msg.sender] = countryId;
}
​
function withdraw() external winnerSet {
...
- if (keccak256(abi.encodePacked(userToCountry[msg.sender])) != keccak256(abi.encodePacked(winner))) {
+ if (userCountryId[msg.sender] != winnerCountryId) {
revert didNotWin();
}
}
Updates

Appeal created

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

Duplicate registration through `joinEvent`

Support

FAQs

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

Give us feedback!