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

The ERC4626::withdraw function can be used to withdraw funds at any time, potential MEV back-run.

alexscherbatyuk

Root + Impact

Description

  • The BriVault contract, building on ERC4626, restricts withdrawals after the event starts via cancelParticipation, which refunds full staked amounts only before eventStartDate. Post-event, after the owner calls setWinner at or after eventEndDate, non-winning participants should have their funds locked to maintain the reward pool integrity for winners, with withdrawals or claims handled through dedicated functions.

  • However, the standard ERC4626::withdraw function remains unoverridden and publicly accessible, allowing any user to withdraw assets based on their share balance at any time, including after setWinner if their chosen team is not the winner, bypassing protocol-specific locks and enabling premature exits that undermine the tournament's reward distribution.

// Root cause in the codebase with @> marks to highlight the relevant section
@> function withdraw(uint256 assets, address receiver, address owner) public virtual returns (uint256) {
uint256 maxAssets = maxWithdraw(owner);
if (assets > maxAssets) {
revert ERC4626ExceededMaxWithdraw(owner, assets, maxAssets);
}
​
uint256 shares = previewWithdraw(assets);
_withdraw(_msgSender(), receiver, owner, assets, shares);
​
return shares;
}

Risk

Likelihood:

  • Owner executes setWinner after eventEndDate to conclude the tournament.

  • Not-satisfied non-winning users or MEV bots detect the transaction and call withdraw immediately to extract funds before reward claims.

Impact:

  • Reward pool for winners is depleted by unauthorized withdrawals, resulting in reduced or zero payouts and unfair distribution.

  • Protocol's post-event logic is circumvented, leading to potential disputes, loss of user trust, and incentives for exploitative behavior like backrunning winner announcements.

Proof of Concept

Add the following code snippet to the briVault.t.sol test file.This test verifies that user can withdraw funds calling ERC4626::withdraw after ERC4626::setWinner.

function test_withdrawAfterSetWinner() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
uint256 user1Shares = briVault.deposit(5 ether, user1);
uint256 balanceBeforuser1 = mockToken.balanceOf(user1);
​
console.log("user1Shares: ", user1Shares);
console.log("balanceBeforuser1: ", balanceBeforuser1);
vm.stopPrank();
​
vm.startPrank(owner);
vm.warp(eventEndDate + 1);
briVault.setWinner(10);
vm.stopPrank();
​
vm.startPrank(user1);
briVault.withdraw(briVault.balanceOf(user1), user1, user1);
uint256 balanceAfteruser1 = mockToken.balanceOf(user1);
console.log("userBalanceAfterWithdrawl: ", balanceAfteruser1);
vm.stopPrank();
​
assertGe(balanceAfteruser1, balanceBeforuser1, "Withdraw is successful, balance has increased");
}

Recommended Mitigation

Possible mitigation to override the standard function ERC4626::withdraw with similar to cancelParticipation logic or revert, preserving ERC4626 standard withdraw arguments.

+ function withdraw(uint256 assets, address receiver, address owner) public override returns (uint256) {
+ if (block.timestamp >= eventStartDate) {
+ revert eventStarted();
+ }
+
+ uint256 refundAmount = stakedAsset[msg.sender];
+
+ if (assets > refundAmount) {
+ revert limiteExceede();
+ }
+
+ uint256 amountToWithdraw = refundAmount - assets;
+
+ stakedAsset[msg.sender] = refundAmount - assets;
+
+ uint256 shares = _convertToShares(amountToWithdraw);
+
+ _burn(msg.sender, shares);
+
+ IERC20(asset()).safeTransfer(msg.sender, amountToWithdraw);
+
+ return amountToWithdraw;
+ }
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!