Root + Impact
Description
The cancelParticipation() function in briVault.sol allows users to cancel their participation and receive a refund before the event starts. However, the function fails to properly clean up the state variables that were modified by joinEvent().
Specifically, cancelParticipation() burns the user's shares and refunds their deposit, but it does NOT:
Remove the user's address from the usersAddress array
Decrement numberOfParticipants
Subtract the user's shares from totalParticipantShares
This causes permanent state corruption that affects the payout calculations for all winners.
function cancelParticipation() public {
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);
}
The inflated totalParticipantShares value is later used in the withdraw() function to calculate payouts:
function withdraw() external winnerSet {
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
}
Note: The totalWinnerShares is calculated by _getWinnerShares(), which sums up shares from the usersAddress array. If cancelled users remain in the array, their shares are incorrectly included in the calculation.
Risk
Likelihood: High
The vulnerability can be triggered by any user who joins and then cancels their participation. This is normal, expected user behavior, not a malicious attack. The issue can also be triggered accidentally by legitimate users who change their mind before the event starts.
Impact: High
The consequences are:
Incorrect payouts for all winners: The inflated totalParticipantShares causes the payout formula to distribute funds incorrectly
Permanent state corruption: Once a user cancels, the state variables remain permanently incorrect with no way to fix them
Compounding effect: Multiple cancellations compound the error, making payouts increasingly inaccurate
Can be combined with H-01: An attacker can join multiple times, then cancel, amplifying the state corruption
This results in direct fund loss for winners, who receive less than they are entitled to.
Proof of Concept
The exploit was confirmed using a Foundry test that demonstrates the state remaining corrupted after a user cancels their participation.
-
Setup:
The BriVault contract is deployed with a tournament event
User1 deposits 10 ETH and joins the event
User2 deposits 10 ETH and joins the event
-
Attack/Normal Behavior:
-
Result:
User2's shares are burned: balanceOf(user2) = 0
User2's deposit is refunded
BUT: numberOfParticipants remains 2 (should be 1)
BUT: totalParticipantShares remains ~19.7e18 (should be ~9.85e18)
BUT: User2's address remains in usersAddress array
Supporting Code:
pragma solidity ^0.8.24;
import {Test, console} from "forge-std/Test.sol";
import {BriVault} from "../src/briVault.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {MockERC20} from "./MockErc20.t.sol";
contract BriVaultExploits is Test {
BriVault public briVault;
MockERC20 public mockToken;
address public owner;
address public user1;
address public user2;
uint256 public eventStartDate;
uint256 public eventEndDate;
function setUp() public {
owner = makeAddr("owner");
user1 = makeAddr("user1");
user2 = makeAddr("user2");
mockToken = new MockERC20("Mock Token", "MTK");
eventStartDate = block.timestamp + 2 days;
eventEndDate = block.timestamp + 31 days;
vm.prank(owner);
briVault = new BriVault(
IERC20(address(mockToken)),
150,
eventStartDate,
makeAddr("feeAddress"),
0.0002 ether,
eventEndDate
);
mockToken.mint(user1, 1000 ether);
mockToken.mint(user2, 1000 ether);
}
function testExploit_CancelParticipationDoesntCleanup() public {
console.log("\n=== EXPLOIT: cancelParticipation() State Corruption ===");
vm.startPrank(user1);
mockToken.approve(address(briVault), 10 ether);
uint256 shares1 = briVault.deposit(10 ether, user1);
briVault.joinEvent(0);
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 10 ether);
uint256 shares2 = briVault.deposit(10 ether, user2);
briVault.joinEvent(1);
vm.stopPrank();
console.log("\n--- State After Both Users Join ---");
console.log("numberOfParticipants:", briVault.numberOfParticipants());
console.log("totalParticipantShares:", briVault.totalParticipantShares());
vm.prank(user2);
briVault.cancelParticipation();
console.log("\n--- State After User2 Cancels ---");
console.log("User2 shares (should be 0):", briVault.balanceOf(user2));
console.log("numberOfParticipants (should be 1):", briVault.numberOfParticipants());
console.log("totalParticipantShares (should be ~9.85e18):", briVault.totalParticipantShares());
assertEq(briVault.balanceOf(user2), 0, "User2 shares should be burned");
assertEq(briVault.numberOfParticipants(), 2, "numberOfParticipants NOT decremented");
assertEq(briVault.totalParticipantShares(), shares1 + shares2, "totalParticipantShares NOT decremented");
console.log("\n[+] EXPLOIT CONFIRMED: State variables remain corrupted after cancel");
console.log(" - numberOfParticipants: 2 (should be 1)");
console.log(" - totalParticipantShares: inflated by cancelled user's shares");
}
}
Test Results:
Test: testExploit_CancelParticipationDoesntCleanup()
Status: PASS
Gas Used: 727,595
Logs:
=== EXPLOIT: cancelParticipation() State Corruption ===
--- State After Both Users Join ---
numberOfParticipants: 2
totalParticipantShares: 19700000000000000000
--- State After User2 Cancels ---
User2 shares (should be 0): 0
numberOfParticipants (should be 1): 2
totalParticipantShares (should be ~9.85e18): 19700000000000000000
[+] EXPLOIT CONFIRMED: State variables remain corrupted after cancel
- numberOfParticipants: 2 (should be 1)
- totalParticipantShares: inflated by cancelled user's shares
Recommended Mitigation
The cancelParticipation() function must properly clean up all state variables that were modified by joinEvent().
function cancelParticipation() public {
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);
+ // Remove user from usersAddress array
+ for (uint256 i = 0; i < usersAddress.length; i++) {
+ if (usersAddress[i] == msg.sender) {
+ usersAddress[i] = usersAddress[usersAddress.length - 1];
+ usersAddress.pop();
+ break;
+ }
+ }
+
+ // Decrement participant counters
+ numberOfParticipants--;
+ totalParticipantShares -= shares;
}
Alternative Approach (More Gas Efficient):
Instead of removing from the array, track cancelled users in a mapping and skip them in _getWinnerShares():
+ mapping(address => bool) public hasCancelled;
function cancelParticipation() public {
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);
+ // Mark user as cancelled
+ hasCancelled[msg.sender] = true;
+
+ // Decrement participant counters
+ numberOfParticipants--;
+ totalParticipantShares -= shares;
}
function _getWinnerShares() internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i) {
address user = usersAddress[i];
+ if (hasCancelled[user]) continue; // Skip cancelled users
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}
Both approaches ensure that cancelled users do not affect the payout calculations.
