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

The `ERC4626::redeem` function allows withdrawal of funds at any time, front-running is possible to withdraw before winner set by owner if chosen country/team is not winner.

alexscherbatyuk

Root + Impact

Description

  • The BriVault contract extends ERC4626, allowing users to deposit assets for tournament participation via deposit, join an event by selecting a country via joinEvent, and have the owner set the winner post-event via setWinner. After the winner is set, users who voted for the winning country should claim rewards proportionally from the pool, while non-winners should have their funds locked or handled per protocol rules until resolution.

  • However, the standard ERC4626::redeem function is not overridden, enabling any user to redeem their shares for underlying assets at any time, including after setWinner is called, even if their chosen country/team is not the winner. This allows non-winning voters to frontrun or immediately withdraw, reducing the pool available for winners.

// Root cause in the codebase with @> marks to highlight the relevant section
/// @inheritdoc IERC4626
@> function redeem(uint256 shares, address receiver, address owner) public virtual returns (uint256) {
uint256 maxShares = maxRedeem(owner);
if (shares > maxShares) {
revert ERC4626ExceededMaxRedeem(owner, shares, maxShares);
}
​
uint256 assets = previewRedeem(shares);
_withdraw(_msgSender(), receiver, owner, assets, shares);
​
return assets;
}

Risk

Likelihood:

  • Owner calls setWinner after eventEndDate to finalize the tournament.

  • Users monitor the transaction and immediately call redeem post-winner announcement if not aligned with the winner.

Impact:

  • Protocol's reward distribution is undermined, as the winning pool diminishes due to premature redemptions by non-winners.

  • Core tournament integrity is compromised, leading to disputes, reduced incentives for participation, and potential fund imbalances.

Proof of Concept

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

function test_redeemAfterSetWinner() 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);
mockToken.approve(address(briVault), 5 ether);
ERC4626(briVault).redeem(user1Shares, user1, user1);
uint256 balanceAfteruser1 = mockToken.balanceOf(user1);
console.log("userBalanceAfterRedeem: ", balanceAfteruser1);
vm.stopPrank();
​
assertGe(balanceAfteruser1, balanceBeforuser1, "Redeem is successful balance has increased");
}

Recommended Mitigation

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

...
+ error RedeemNotAllowed();
...
+ function redeem(uint256 shares, address receiver, address owner) public override returns (uint256) {
+ revert RedeemNotAllowed();
+ }
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!