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

ERC4626 Mint After Joining Triggers Permanent Withdrawal DoS

galer ah

Root + Impact

Description

Because BriVault leaves ERC4626 mint unrestricted, a participant can enlarge their share balance after joining without paying additional fees or updating team allocations. userSharesToCountry is captured during joinEvent (src/briVault.sol:260-262) and never refreshed. When setWinner sums the cached values, the denominator is smaller than the live winning supply. The bespoke withdraw() then computes assetToWithdraw using the bloated numerator, causing SafeERC20.safeTransfer to revert.

function joinEvent(uint256 countryId) public {
...
@> uint256 participantShares = balanceOf(msg.sender);
@> userSharesToCountry[msg.sender][countryId] = participantShares;
}
​
function withdraw() external winnerSet {
uint256 assetToWithdraw = Math.mulDiv(
@> shares,
@> finalizedVaultAsset,
@> totalWinnerShares
);
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
}

Risk

Likelihood: High – calling mint is a single transaction and requires no new permissions.

Impact:

  • Winners who (intentionally or accidentally) mint after joining cause all withdrawals for that team to revert.

  • Attacker avoids participation fees while locking the entire prize pool.

Proof of Concept

  1. User deposits and joins before the event starts.

  2. After eventStartDate, user calls briVault.mint(briVault.balanceOf(user), user).

  3. Owner finalizes with setWinner(team).

  4. Calling withdraw() reverts with "ERC20: transfer amount exceeds balance".
    (Captured in test/BriVaultAttack.t.sol:179 as testMintAfterJoinBreaksWinnerWithdrawal.)

function testMintAfterJoinBreaksWinnerWithdrawal() public {
briVault.deposit(FIRST_DEPOSIT, winner);
briVault.joinEvent(0);
vm.warp(eventStartDate + 1);
​
uint256 shares = briVault.balanceOf(winner);
briVault.mint(shares, winner); // doubles balance without updating mapping
​
vm.prank(owner);
briVault.setWinner(0);
​
vm.prank(winner);
vm.expectRevert("ERC20: transfer amount exceeds balance");
briVault.withdraw();
}

Recommended Mitigation

  1. Override/disable ERC4626 mint, withdraw, and redeem unless they honour the tournament lifecycle and fee model.

  2. Keep userSharesToCountry synchronized with live balances or compute allocations from current shares when withdrawing.

  3. Add invariant tests ensuring assetToWithdraw never exceeds the vault’s actual balance.

Proposed patch (Solidity-like pseudocode):

function mint(uint256 shares, address receiver) public override returns (uint256) {
- return super.mint(shares, receiver);
+ revert ERC4626Disabled();
}
​
function _afterShareChange(address user) internal {
- // no-op today
+ uint256 country = countryOf[user];
+ if (country != INVALID) {
+ userSharesToCountry[user][country] = balanceOf(user);
+ }
}
​
function deposit(uint256 assets, address receiver) public override returns (uint256 shares) {
@@
- _mint(msg.sender, participantShares);
+ _mint(receiver, participantShares);
+ _afterShareChange(receiver);
}
Updates

Appeal created

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

Unrestricted ERC4626 functions

Support

FAQs

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

Give us feedback!