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

Deposit and withdraw break ERC4626 interface

fredo182

Deposit and withdraw break ERC4626 interface

Description

Under the ERC4626 standard, the deposit() and withdraw() functions must emit the canonical Deposit and Withdraw events defined in the interface to ensure interoperability with aggregators, vault routers, and accounting systems.

However, in the current implementation, both deposit() and withdraw() are overridden but emit custom events instead of the defined. This breaks compatibility with ERC-4626 frontends, indexers, and external integrators that rely on the standard event signatures.

Risk

Likelihood: High
Occurs on every deposit and withdrawal since non-standard events are emitted.

Impact: Low
Off-chain systems relying on ERC-4626 events will not detect deposit/withdrawals.

Proof of Concept

For deposit function:

function test_erc4626_event_is_missing() {
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
vm.expectEmit(true, true, false, false);
emit BriVault.deposited(user1, 1 ether);
briVault.deposit(1 ether, user1);
vm.stopPrank();
}

For withdrawal function:

function test_erc4626_event_is_missing() {
vm.prank(user1);
vm.expectEmit(true, true, false, false);
emit BriVault.Withdraw(user1, 0.985 ether);
briVault.withdraw();
}

Recommended Mitigation

Emit the ERC4626 events along with any custom event

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
​
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
uint256 fee = _getParticipationFee(assets);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
​
uint256 stakeAsset = assets - fee;
​
stakedAsset[receiver] = stakeAsset;
​
uint256 participantShares = _convertToShares(stakeAsset);
​
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
​
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
​
_mint(msg.sender, participantShares);
​
emit deposited(receiver, stakeAsset);
+ emit Deposit(msg.sender, receiver, assets, participantShares);
​
return participantShares;
}
​
...
​
​
function withdraw() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
​
if (keccak256(abi.encodePacked(userToCountry[msg.sender])) != keccak256(abi.encodePacked(winner))) {
revert didNotWin();
}
uint256 shares = balanceOf(msg.sender);
​
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
​
_burn(msg.sender, shares);
​
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
​
- emit Withdraw(msg.sender, assetToWithdraw);
+ emit Withdraw(msg.sender, receiver, owner, assets, shares);
}
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!