Part 2

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

Potential Overflow in Deposit Cap Check

Summary

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

The ZlpVault contract has a potential overflow vulnerability in the logic used to check the deposit cap when calculating the maximum depositable assets. The vulnerability lies in the unchecked subtraction of totalAssetsCached from depositCap. If a malicious actor directly manipulates the contract state or the underlying asset balance, this could lead to incorrect values and bypass the intended deposit cap restrictions.

Vulnerability Details

/// @notice Returns the maximum amount of assets that can be deposited into the ZLP Vault, taking into account the
/// configured deposit cap.
function maxDeposit(address) public view override returns (uint256 maxAssets) {
// load the zlp vault storage pointer
ZlpVaultStorage storage zlpVaultStorage = _getZlpVaultStorage();
// cache the market making engine contract
IMarketMakingEngine marketMakingEngine = IMarketMakingEngine(zlpVaultStorage.marketMakingEngine);
// get the vault's deposit cap
uint128 depositCap = marketMakingEngine.getDepositCap(zlpVaultStorage.vaultId);
// cache the vault's total assets
uint256 totalAssetsCached = totalAssets();
// underflow check here would be redundant
unchecked {
// we need to ensure that depositCap > totalAssets, otherwise, a malicious actor could grief deposits by
// sending assets directly to the vault contract and bypassing the deposit cap
maxAssets = depositCap > totalAssetsCached ? depositCap - totalAssetsCached : 0;
}
}

Unchecked Arithmetic:

The subtraction of totalAssetsCached from depositCap is performed inside an unchecked block. While it is true that depositCap > totalAssetsCached is checked, this relies on external state that might be manipulated outside of the function.

External Asset Manipulation:

The totalAssets() function calculates the contract's total asset balance, which could include tokens sent directly to the contract. If a malicious actor sends tokens directly to the contract without going through the deposit flow, the totalAssetsCached value could exceed depositCap. This could cause the subtraction to revert or lead to incorrect calculations.

Misaligned State:

If the depositCap is updated externally but is not synchronized with the vault's actual asset balance (totalAssets), this could lead to unexpected behavior, such as incorrectly restricting deposits or allowing excess deposits.

Exploitation Scenario:

Griefing Deposits: A malicious actor could directly transfer tokens to the vault, causing totalAssetsCached to exceed the depositCap. This could result in the maxDeposit function returning zero, effectively blocking legitimate deposits.

Bypassing Cap: If the depositCap logic is not robustly enforced across all relevant functions, a malicious actor might bypass the intended cap by exploiting discrepancies between the calculated totalAssetsCached and the actual balance.

Impact

Denial of Service: Legitimate users could be blocked from depositing assets into the vault due to manipulated state, leading to a loss of utility.

Loss of Funds Control: Incorrect cap enforcement could result in deposits exceeding the intended limit, potentially causing financial or operational issues for the vault.

Tools Used

Manual Review

Recommendations

Replace the unchecked block with safe arithmetic checks to prevent unexpected overflows or underflows. Solidity’s default behavior includes these checks, which ensures safety at the cost of minor gas overhead.

function maxDeposit(address) public view override returns (uint256 maxAssets) {
ZlpVaultStorage storage zlpVaultStorage = _getZlpVaultStorage();
IMarketMakingEngine marketMakingEngine = IMarketMakingEngine(zlpVaultStorage.marketMakingEngine);
uint128 depositCap = marketMakingEngine.getDepositCap(zlpVaultStorage.vaultId);
uint256 totalAssetsCached = totalAssets();
// Ensure safe arithmetic
if (depositCap > totalAssetsCached) {
maxAssets = depositCap - totalAssetsCached;
} else {
maxAssets = 0;
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
5 months ago
inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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