BriVault

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

# The `joinEvent` function fails to prevent users from being repeatedly added to `usersAddress`, which can dilute the winners' withdrawal amounts. ## Description * When users call the `joinEvent` function to participate in the event, the protocol us

Users Can Easily Withdraw Funds After Any Deposit

Description

  • The protocol documentation states: Users can deposit an ERC20 asset to bet on a team. After the tournament ends, winners will share the prize pool proportionally based on their deposit amount.

  • In other words, users who bet on the losing national team will not receive any rewards.

  • However, since the protocol inherits from ERC4626 and does not restrict users from calling the original withdraw and redeem functions, users can withdraw funds unimpeded. This clearly violates the protocol's original intent.

// Fails to override ERC4626's withdraw function
function withdraw() external winnerSet {
// Original code..
}
// Fails to override ERC4626's redeem function

Risk

Likelihood:

  • Once a user deposits funds, they can definitely withdraw them easily at any time.

Impact:

  • Breaks the protocol's core economic model and incentive mechanism: Users can essentially bypass the "only winners share the prize pool" rule and withdraw funds anytime.

  • Invalidates the prize pool mechanism: Since losing users can withdraw funds at will, the actual prize pool available to winners will be far lower than expected, or even zero, rendering the entire betting reward mechanism ineffective.

  • Collapses the protocol's economic incentives: Without the core element of "betting risk," the protocol cannot effectively encourage user participation. Rational users will bet on all teams or withdraw funds frequently, leading to the protocol's failure to operate normally.

  • Inflated TVL (Total Value Locked): The apparent deposit scale does not truly reflect the protocol's actual locked value, as funds can flow out at any time, undermining the protocol's financial sustainability assumptions.

Proof of Concept

  • Add the following function to test/BriVaultTest.t.sol and run forge test --mt test__usingOriginalFunction -vv:

function test__usingOriginalFunction() public {
uint256 balanceOfUser1_BeforeWithdraw;
uint256 balanceOfUser1_RedeemWithdraw;
vm.startPrank(user1);
// User1 deposits all their funds into the vault
mockToken.approve(address(briVault), 20 ether);
briVault.deposit(20 ether, user1);
balanceOfUser1_BeforeWithdraw = mockToken.balanceOf(user1);
// User1 easily uses the withdraw function to extract any amount (demonstration with partial withdrawal)
briVault.withdraw(briVault.maxWithdraw(user1)/10, user1, user1);
vm.assertTrue(mockToken.balanceOf(user1) > balanceOfUser1_BeforeWithdraw);
balanceOfUser1_RedeemWithdraw = mockToken.balanceOf(user1);
// User1 easily uses the redeem function to extract any amount (demonstration with partial withdrawal)
briVault.redeem(briVault.maxRedeem(user1)/10, user1, user1);
vm.assertTrue(mockToken.balanceOf(user1) > balanceOfUser1_RedeemWithdraw);
// User1 selects the national team with index 10
briVault.joinEvent(10);
balanceOfUser1_BeforeWithdraw = mockToken.balanceOf(user1);
// User1 easily uses the withdraw function to extract any amount (demonstration with partial withdrawal)
briVault.withdraw(briVault.maxWithdraw(user1)/10, user1, user1);
vm.assertTrue(mockToken.balanceOf(user1) > balanceOfUser1_BeforeWithdraw);
balanceOfUser1_RedeemWithdraw = mockToken.balanceOf(user1);
// User1 easily uses the redeem function to extract any amount (demonstration with partial withdrawal)
briVault.redeem(briVault.maxRedeem(user1)/10, user1, user1);
vm.assertTrue(mockToken.balanceOf(user1) > balanceOfUser1_RedeemWithdraw);
vm.stopPrank();
// Fast-forward timestamp to after the event ends
vm.warp(eventEndDate + 1);
// Admin declares the winning national team
vm.prank(owner);
briVault.setWinner(10);
vm.startPrank(user1);
balanceOfUser1_BeforeWithdraw = mockToken.balanceOf(user1);
// User1 easily uses the withdraw function to extract any amount (demonstration with partial withdrawal)
briVault.withdraw(briVault.maxWithdraw(user1)/10, user1, user1);
vm.assertTrue(mockToken.balanceOf(user1) > balanceOfUser1_BeforeWithdraw);
balanceOfUser1_RedeemWithdraw = mockToken.balanceOf(user1);
// User1 easily uses the redeem function to extract any amount (demonstration with partial withdrawal)
briVault.redeem(briVault.maxRedeem(user1)/10, user1, user1);
vm.assertTrue(mockToken.balanceOf(user1) > balanceOfUser1_RedeemWithdraw);
vm.stopPrank();
}
  • Console output:

Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] test__usingOriginalFunction() (gas: 570155)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.44ms (754.30µs CPU time)

Recommended Mitigation

  • Fully override ERC4626's withdraw and redeem functions by adding the following code:

+ /**
+ * @dev Overrides ERC4626's withdraw function to fully disable it.
+ * @param assets Amount of assets attempting to be withdrawn (this call will always fail)
+ * @param receiver Address of the recipient
+ * @param owner Address of the asset owner
+ */
+ function withdraw(
+ uint256 assets,
+ address receiver,
+ address owner
+ ) public override returns (uint256) {
+ revert("BriVault: Use 'withdraw()' after winner is set. Direct withdrawals are disabled.");
+ }
+ /**
+ * @dev Overrides ERC4626's redeem function to fully disable it.
+ * @param shares Amount of shares attempting to be redeemed (this call will always fail)
+ * @param receiver Address of the recipient
+ * @param owner Address of the asset owner
+ */
+ function redeem(
+ uint256 shares,
+ address receiver,
+ address owner
+ ) public override returns (uint256) {
+ revert("BriVault: Use 'withdraw()' after winner is set. Direct redemptions are 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!