Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: medium
Invalid

ERC4626 Incompatibility Risks

Summary

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/39e33b2f6b3890573bb1affc41a7e520277ceb2c/src/zlp/ZlpVault.sol#L123

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/39e33b2f6b3890573bb1affc41a7e520277ceb2c/src/zlp/ZlpVault.sol#L128

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/39e33b2f6b3890573bb1affc41a7e520277ceb2c/src/zlp/ZlpVault.sol#L133

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/39e33b2f6b3890573bb1affc41a7e520277ceb2c/src/zlp/ZlpVault.sol#L147

The ZlpVault contract extends ERC4626Upgradeable but includes several areas where the implementation deviates from or could cause incompatibility with the ERC4626 standard. These issues could result in unexpected behavior when interacting with other systems or protocols that expect ERC4626 compliance. In particular, the custom logic in the deposit, mint, withdraw, and redeem functions can cause interoperability issues.

Vulnerability Details

/// @inheritdoc ERC4626Upgradeable
function deposit(uint256 assets, address receiver) public override onlyMarketMakingEngine returns (uint256) {
return super.deposit(assets, receiver);
}
/// @inheritdoc ERC4626Upgradeable
function mint(uint256 shares, address receiver) public override onlyMarketMakingEngine returns (uint256) {
return super.mint(shares, receiver);
}
/// @inheritdoc ERC4626Upgradeable
function withdraw(
uint256 assets,
address receiver,
address owner
) public override onlyMarketMakingEngine returns (uint256) {
return super.withdraw(assets, receiver, owner);
}
/// @inheritdoc ERC4626Upgradeable
function redeem(
uint256 shares,
address receiver,
address owner
) public override onlyMarketMakingEngine returns (uint256) {
return super.redeem(shares, receiver, owner);
}

Deviation from ERC4626 Expected Behavior:

ERC4626 Standard: The standard expects certain functions (deposit, mint, withdraw, and redeem) to be callable by any account, but in ZlpVault, these functions are restricted to the MarketMakingEngine through the onlyMarketMakingEngine modifier. This restriction violates the standard and could prevent legitimate users or contracts from interacting with the vault as intended.

ERC4626 Compatibility: The ERC4626 standard defines strict guidelines for how these functions should work, including who can call them and how the accounting for assets should be handled. By modifying the access control and relying on a specific external contract (MarketMakingEngine), ZlpVault may not be compatible with ERC4626-compliant systems or protocols.

Non-Standard Behavior:

The maxDeposit, maxMint, maxWithdraw, and maxRedeem functions in ZlpVault are overridden and modified to enforce limits based on the MarketMakingEngine. However, these limits are not part of the ERC4626 specification, meaning that other ERC4626-compliant systems may not expect these restrictions.

The custom maxDeposit and maxMint implementations could cause conflicts with systems that expect a simpler, unrestricted deposit and mint mechanism.

Inconsistent Handling of Assets and Shares:

In the ERC4626Upgradeable implementation, assets and shares are directly linked with standardized conversion functions. However, the ZlpVault contract introduces custom logic to convert between assets and shares using the MarketMakingEngine, potentially causing discrepancies when interacting with other ERC4626-compliant systems or tools that assume the standard conversion methods.

Exploitation Scenario:

Interoperability Issues: The modified logic for access control and the custom cap enforcement mechanism could make it difficult for other contracts, external users, or decentralized applications (dApps) to integrate with ZlpVault. This could lead to failed transactions, improper asset accounting, or unexpected behavior when interacting with decentralized finance (DeFi) protocols or automated systems that expect ERC4626-compliant vaults.

Inability to Migrate or Integrate with ERC4626 Ecosystem: If ZlpVault is designed to be used within a broader ecosystem that expects ERC4626 compliance, the deviations from the standard could prevent it from being integrated with other systems like yield aggregators, lending platforms, or decentralized exchanges.

Impact

Loss of Interoperability: The contract’s restrictions and custom logic could prevent it from being used in other ERC4626-compliant ecosystems, limiting its flexibility and integration with decentralized finance (DeFi) platforms.

Potential Loss of Funds: Users or other contracts may encounter issues interacting with the vault, such as being unable to deposit, withdraw, or mint shares, leading to potential loss of funds or failures in DeFi protocols that rely on it.

Complexity and Maintenance Challenges: The custom logic may make the contract harder to maintain and upgrade, as well as introducing potential risks when trying to integrate with future versions of the ERC4626 standard or other compliant systems.

Tools Used

Manual Review

Recommendations

Align with ERC4626 Standards

To improve compatibility with the ERC4626 standard and ensure easier integration with DeFi protocols, consider the following changes:

Remove onlyMarketMakingEngine Modifier
The deposit, mint, withdraw, and redeem functions should be callable by any user, not just the MarketMakingEngine. Modify these functions to remove the onlyMarketMakingEngine restriction, as it diverges from ERC4626’s expectations:

function deposit(uint256 assets, address receiver) public override returns (uint256) {
return super.deposit(assets, receiver);
}
function mint(uint256 shares, address receiver) public override returns (uint256) {
return super.mint(shares, receiver);
}
function withdraw(uint256 assets, address receiver, address owner) public override returns (uint256) {
return super.withdraw(assets, receiver, owner);
}
function redeem(uint256 shares, address receiver, address owner) public override returns (uint256) {
return super.redeem(shares, receiver, owner);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
5 months ago
inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.