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

Unrestricted share redemption bypasses tournament logic

fredo182

Unrestricted share redemption bypasses tournament logic

Description

An ERC4626 vault defines controlled asset withdrawal via the redeem() and withdrawal() functions, which calculate the proportionate share of assets a holder can retrieve based on their ownership and vault's total balance.

In this vault, the intended flow is that users may only withdraw assets through the customer withdraw() function after the event has ended and only if they belong to the winning country.

However, the inherited ERC4626.redeem() function remains publicly callable and is not overridden in BriVault. This allows any share holder to directly call redeem() on the contract, bypassing the tournament restrictions and accessing their proportionate share of the vault's assets regardless of event state, winner, or eligibility. This completely breaks the game logic and incentive model.

Risk

Likelihood: High
The condition occurs whenever a participant or even a non-participant holding shares calls redeem() directly, which is always possible.

Impact: High
All users can freely withdraw underlying assets before the event ends, undermining the entire tournament staking mechanism. As such, the vault's balance can be drained, preventing legitimate winners from receiving rewards and breaking trust and accounting consistency.

Proof of Concept

The following test case proves the unrestricted redemptions from a user who does not belong in the winners set.

function test_protocol_allows_unrestricted_redemptions() public {
assertEq(mockToken.balanceOf(user1), 20 ether);
assertEq(briVault.balanceOf(user1), 0);
​
vm.startPrank(user1);
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(1 ether, user1);
briVault.joinEvent(5); // User1 selects "Ecuador"
vm.stopPrank();
assertEq(mockToken.balanceOf(user1), 19 ether);
assertEq(briVault.balanceOf(user1), 0.985 ether); // 1 ether - 1.5% fee
​
vm.warp(eventEndDate + 1 days);
​
vm.startPrank(owner);
briVault.setWinner(4); // "Brazil" is the winner
vm.stopPrank();
assertEq(briVault.getWinner(), "Brazil");
​
vm.startPrank(user1);
briVault.redeem(0.985 ether, user1, user1);
vm.stopPrank();
uint256 userBalanceAfterRedeem = mockToken.balanceOf(user1);
uint256 userSharesAfterRedeem = briVault.balanceOf(user1);
assertEq(mockToken.balanceOf(user1), 19.985 ether);
assertEq(briVault.balanceOf(user1), 0 ether);
}

Recommended Mitigation

The redeem() function can be overriden and throw, forcing participants to withdraw only through withdraw().

+ function redeem(uint256, address, address) public pure override returns (uint256) {
+ revert("Direct redeem disabled; use withdraw()");
+ }
Updates

Appeal created

bube Lead Judge 21 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!