Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: high
Valid

Vault's debt can never be settled as Vault::marketsRealizedDebtUsd is always returns zero

Summary

Due to an overly aggressive guard clause in Market::getVaultAccumulatedValues, creditdelegationbranch::lastVaultDistributedRealizedDebtUsdPerShare is never updated from its initial zero value. As a result, subsequent accumulation events always calculate a zero change, leading to incorrect debt distributions across connected markets.

Vulnerability Details

A key function in the zaros protocol is Vault::recalculateVaultsCreditCapacity. This function is used to update the vaults credit capacity which updates a host of state variables that are key for other functions to calculate accurately. See below:

/// @notice Recalculates the latest credit capacity of the provided vaults ids taking into account their latest
/// assets and debt usd denonimated values.
/// @dev We use a `uint256` array because a market's connected vaults ids are stored at a `EnumerableSet.UintSet`.
/// @dev We assume this function's caller checks that connectedMarketsIdsCache > 0.
/// @param vaultsIds The array of vaults ids to recalculate the credit capacity.
// todo: check where we're messing with the `continue` statement
function recalculateVaultsCreditCapacity(uint256[] memory vaultsIds) internal {
for (uint256 i; i < vaultsIds.length; i++) {
// uint256 -> uint128
uint128 vaultId = vaultsIds[i].toUint128();
// load the vault storage pointer
Data storage self = load(vaultId);
// make sure there are markets connected to the vault
uint256 connectedMarketsConfigLength = self.connectedMarkets.length;
if (connectedMarketsConfigLength == 0) continue;
// loads the connected markets storage pointer by taking the last configured market ids uint set
EnumerableSet.UintSet storage connectedMarkets = self.connectedMarkets[connectedMarketsConfigLength - 1];
// cache the connected markets ids to avoid multiple storage reads, as we're going to loop over them twice
// at `_recalculateConnectedMarketsState` and `_updateCreditDelegations`
uint128[] memory connectedMarketsIdsCache = new uint128[](connectedMarkets.length());
// update vault and credit delegation weight
updateVaultAndCreditDelegationWeight(self, connectedMarketsIdsCache);
// iterate over each connected market id and distribute its debt so we can have the latest credit
// delegation of the vault id being iterated to the provided `marketId`
(
uint128[] memory updatedConnectedMarketsIdsCache,
SD59x18 vaultTotalRealizedDebtChangeUsdX18,
SD59x18 vaultTotalUnrealizedDebtChangeUsdX18,
UD60x18 vaultTotalUsdcCreditChangeX18,
UD60x18 vaultTotalWethRewardChangeX18
) = _recalculateConnectedMarketsState(self, connectedMarketsIdsCache, true);
// gas optimization: only write to storage if values have changed
//
// updates the vault's stored unsettled realized debt distributed from markets
if (!vaultTotalRealizedDebtChangeUsdX18.isZero()) {
self.marketsRealizedDebtUsd = sd59x18(self.marketsRealizedDebtUsd).add(
vaultTotalRealizedDebtChangeUsdX18
).intoInt256().toInt128();
}
// updates the vault's stored unrealized debt distributed from markets
if (!vaultTotalUnrealizedDebtChangeUsdX18.isZero()) {
self.marketsUnrealizedDebtUsd = sd59x18(self.marketsUnrealizedDebtUsd).add(
vaultTotalUnrealizedDebtChangeUsdX18
).intoInt256().toInt128();
}
// adds the vault's total USDC credit change, earned from its connected markets, to the
// `depositedUsdc` variable
if (!vaultTotalUsdcCreditChangeX18.isZero()) {
self.depositedUsdc = ud60x18(self.depositedUsdc).add(vaultTotalUsdcCreditChangeX18).intoUint128();
}
// distributes the vault's total WETH reward change, earned from its connected markets
if (!vaultTotalWethRewardChangeX18.isZero() && self.wethRewardDistribution.totalShares != 0) {
SD59x18 vaultTotalWethRewardChangeSD59X18 =
sd59x18(int256(vaultTotalWethRewardChangeX18.intoUint256()));
self.wethRewardDistribution.distributeValue(vaultTotalWethRewardChangeSD59X18);
}
// update the vault's credit delegations
(, SD59x18 vaultNewCreditCapacityUsdX18) =
_updateCreditDelegations(self, updatedConnectedMarketsIdsCache, false);
emit LogUpdateVaultCreditCapacity(
vaultId,
vaultTotalRealizedDebtChangeUsdX18.intoInt256(),
vaultTotalUnrealizedDebtChangeUsdX18.intoInt256(),
vaultTotalUsdcCreditChangeX18.intoUint256(),
vaultTotalWethRewardChangeX18.intoUint256(),
vaultNewCreditCapacityUsdX18.intoInt256()
);
}
}

This function calls Market::getVaultAccumulatedValues. See below:

/// @notice Calculates the latest debt, usdc credit and weth reward values a vault is entitled to receive from the
/// market since its last accumulation event.
/// @param self The market storage pointer.
/// @param vaultDelegatedCreditUsdX18 The vault's delegated credit in USD.
/// @param lastVaultDistributedRealizedDebtUsdPerShareX18 The vault's last distributed realized debt per credit
/// share.
/// @param lastVaultDistributedUnrealizedDebtUsdPerShareX18 The vault's last distributed unrealized debt per
/// credit share.
/// @param lastVaultDistributedUsdcCreditPerShareX18 The vault's last distributed USDC credit per credit share.
/// @param lastVaultDistributedWethRewardPerShareX18 The vault's last distributed WETH reward per credit share.
function getVaultAccumulatedValues(
Data storage self,
UD60x18 vaultDelegatedCreditUsdX18,
SD59x18 lastVaultDistributedRealizedDebtUsdPerShareX18,
SD59x18 lastVaultDistributedUnrealizedDebtUsdPerShareX18,
UD60x18 lastVaultDistributedUsdcCreditPerShareX18,
UD60x18 lastVaultDistributedWethRewardPerShareX18
)
internal
view
returns (
SD59x18 realizedDebtChangeUsdX18,
SD59x18 unrealizedDebtChangeUsdX18,
UD60x18 usdcCreditChangeX18,
UD60x18 wethRewardChangeX18
)
{
// calculate the vault's share of the total delegated credit, from 0 to 1
UD60x18 vaultCreditShareX18 = vaultDelegatedCreditUsdX18.div(getTotalDelegatedCreditUsd(self));
// calculate the vault's value changes since its last accumulation
// note: if the last distributed value is zero, we assume it's the first time the vault is accumulating
// values, thus, it needs to return zero changes
realizedDebtChangeUsdX18 = !lastVaultDistributedRealizedDebtUsdPerShareX18.isZero()
? sd59x18(self.realizedDebtUsdPerVaultShare).sub(lastVaultDistributedRealizedDebtUsdPerShareX18).mul(
vaultCreditShareX18.intoSD59x18()
)
: SD59x18_ZERO;
unrealizedDebtChangeUsdX18 = !lastVaultDistributedUnrealizedDebtUsdPerShareX18.isZero()
? sd59x18(self.unrealizedDebtUsdPerVaultShare).sub(lastVaultDistributedUnrealizedDebtUsdPerShareX18).mul(
vaultCreditShareX18.intoSD59x18()
)
: SD59x18_ZERO;
usdcCreditChangeX18 = !lastVaultDistributedUsdcCreditPerShareX18.isZero()
? ud60x18(self.usdcCreditPerVaultShare).sub(lastVaultDistributedUsdcCreditPerShareX18).mul(
vaultCreditShareX18
)
: UD60x18_ZERO;
// TODO: fix the vaultCreditShareX18 flow to multiply by `wethRewardChangeX18`
wethRewardChangeX18 = ud60x18(self.wethRewardPerVaultShare).sub(lastVaultDistributedWethRewardPerShareX18);
}

The bug occurs for 2 reason. Market::getVaultAccumulatedValues has the following line:

realizedDebtChangeUsdX18 = !lastVaultDistributedRealizedDebtUsdPerShareX18.isZero()
? sd59x18(self.realizedDebtUsdPerVaultShare).sub(lastVaultDistributedRealizedDebtUsdPerShareX18).mul(
vaultCreditShareX18.intoSD59x18()
)
: SD59x18_ZERO;

This checks if lastVaultDistributedRealizedDebtUsdPerShareX18 is zero and, if so, returns a zero change for the realized debt calculation. The initial state of lastVaultDistributedRealizedDebtUsdPerShare is zero and so when Vault::recalculateVaultsCreditCapacity is first called, this value returns 0.

However, Vault::_recalculateConnectedMarketsState has the following code block:

if (
ctx.realizedDebtChangeUsdX18.isZero() && ctx.unrealizedDebtChangeUsdX18.isZero()
&& ctx.usdcCreditChangeX18.isZero() && ctx.wethRewardChangeX18.isZero()
) {
continue;
}
// update the vault's state by adding its share of the market's latest state variables
vaultTotalRealizedDebtChangeUsdX18 = vaultTotalRealizedDebtChangeUsdX18.add(ctx.realizedDebtChangeUsdX18);
vaultTotalUnrealizedDebtChangeUsdX18 =
vaultTotalUnrealizedDebtChangeUsdX18.add(ctx.unrealizedDebtChangeUsdX18);
vaultTotalUsdcCreditChangeX18 = vaultTotalUsdcCreditChangeX18.add(ctx.usdcCreditChangeX18);
vaultTotalWethRewardChangeX18 = vaultTotalWethRewardChangeX18.add(ctx.wethRewardChangeX18);
// update the last distributed debt, credit and reward values to the vault's credit delegation to the
// given market id, in order to keep next calculations consistent
creditDelegation.updateVaultLastDistributedValues(
sd59x18(market.realizedDebtUsdPerVaultShare),
sd59x18(market.unrealizedDebtUsdPerVaultShare),
ud60x18(market.usdcCreditPerVaultShare),
ud60x18(market.wethRewardPerVaultShare)
);
}
}

This checks if ctx.realizedDebtChangeUsdX18.isZero() && ctx.unrealizedDebtChangeUsdX18.isZero()&& ctx.usdcCreditChangeX18.isZero() && ctx.wethRewardChangeX18.isZero() are all zero which in the first instance they will be due to the condition in Market::getVaultAccumulatedValues. In this case, it skips the iteration. By skipping the iteration, it misses out on a key function call which is creditDelegation.updateVaultLastDistributedValues. This function updates all state variables in creditdelegation.data with their appropriate data especially with updating market.realizedDebtUsdPerVaultShare which is used to update lastVaultDistributedRealizedDebtUsdPerShareX18 in the function.

Since this value is not updated in creditdelegation.data, lastVaultDistributedRealizedDebtUsdPerShareX18 will remain 0 whenever Vault::recalculateVaultsCreditCapacity is next called. In fact, no matter how many times it is called, lastVaultDistributedRealizedDebtUsdPerShareX18 will be 0. This has additional effects as what this means is that whenever creditdelegationbranch::settlevaultsdeposit is called to settle vaults debt, the vaults debt will always be zero. The key line in the function is:

ctx.vaultUnsettledRealizedDebtUsdX18 = vault.getUnsettledRealizedDebt();

See vault.getUnsettledRealizedDebt:

function getUnsettledRealizedDebt(Data storage self)
internal
view
returns (SD59x18 unsettledRealizedDebtUsdX18)
{
unsettledRealizedDebtUsdX18 =
sd59x18(self.marketsRealizedDebtUsd).add(unary(ud60x18(self.depositedUsdc).intoSD59x18()));
}

Vault::marketsRealizedDebtUsd is calculated with the following line in Vault::recalculateVaultsCreditCapacity:

if (!vaultTotalRealizedDebtChangeUsdX18.isZero()) {
self.marketsRealizedDebtUsd = sd59x18(self.marketsRealizedDebtUsd).add(
vaultTotalRealizedDebtChangeUsdX18
).intoInt256().toInt128();

vaultTotalRealizedDebtChangeUsdX18 is calculated from the following line in Vault::_recalculateconnectedmarketsstate:

vaultTotalRealizedDebtChangeUsdX18 = vaultTotalRealizedDebtChangeUsdX18.add(ctx.realizedDebtChangeUsdX18);

and finally, realizedDebtChangeUsdX18 is calculated in Market::getVaultAccumulatedValues as follows:

realizedDebtChangeUsdX18 = !lastVaultDistributedRealizedDebtUsdPerShareX18.isZero()
? sd59x18(self.realizedDebtUsdPerVaultShare).sub(lastVaultDistributedRealizedDebtUsdPerShareX18).mul(
vaultCreditShareX18.intoSD59x18()
)
: SD59x18_ZERO;

This is how the vault debt maps to Market::realizedDebtUsdPerVaultShare which is the variable we identified that was never updated from 0. As a result, the vault debt is never updated from 0 so whenever creditdelegationbranch::settlevaultsdebt is called, the vault debt will always be 0 even when the vault should be allocated debt.

Proof Of Code (POC)

function test_vaultdebtisneverupdated(
uint128 vaultId,
uint128 assetsToDeposit,
uint128 marketId
)
external
{
vm.stopPrank();
//c configure vaults and markets
VaultConfig memory fuzzVaultConfig = getFuzzVaultConfig(vaultId);
vm.assume(fuzzVaultConfig.asset != address(usdc));
PerpMarketCreditConfig memory fuzzMarketConfig = getFuzzPerpMarketCreditConfig(marketId);
uint256[] memory marketIds = new uint256[](1);
marketIds[0] = fuzzMarketConfig.marketId;
uint256[] memory vaultIds = new uint256[](1);
vaultIds[0] = fuzzVaultConfig.vaultId;
vm.prank(users.owner.account);
marketMakingEngine.connectVaultsAndMarkets(marketIds, vaultIds);
// ensure valid deposit amount
address userA = users.naruto.account;
assetsToDeposit = 1e14;
//c need to deposit twice due to recalculatevaultcreditcapacity not being called in vaultrouterbranch::deposit which is a bug I reported
fundUserAndDepositInVault(userA, fuzzVaultConfig.vaultId, assetsToDeposit);
deal(fuzzVaultConfig.asset, userA, 100e18);
vm.prank(userA);
marketMakingEngine.deposit(fuzzVaultConfig.vaultId, assetsToDeposit, 0, "", false);
deal(fuzzVaultConfig.asset, address(fuzzMarketConfig.engine), 100e18);
//c perp engine deposits credit into market
vm.prank(address(fuzzMarketConfig.engine));
marketMakingEngine.depositCreditForMarket(fuzzMarketConfig.marketId, fuzzVaultConfig.asset, fuzzVaultConfig.depositCap*2);
//c recalculatevaultcreditcapacity is not called in depositcreditformarket which is a bug I reported so i have to call vaultrouterbranch::deposit again to update the credit capacity of the vault
address userB = users.sasuke.account;
deal(fuzzVaultConfig.asset, userB, 100e18);
vm.startPrank(userB);
marketMakingEngine.deposit(fuzzVaultConfig.vaultId, assetsToDeposit, 0, "", false);
vm.stopPrank();
//c proof that marketdebt has been updated
SD59x18 totalmarketdebt = marketMakingEngine.workaround_getTotalMarketDebt(fuzzMarketConfig.marketId);
console.log(totalmarketdebt.unwrap());
//c proof that the realizeddebtpervault
int128 realizeddebtpervaultshare = marketMakingEngine.workaround_getrealizedDebtUsdPerVaultShare(fuzzMarketConfig.marketId);
console.log(realizeddebtpervaultshare);
//c this value should match the realizeddebtpervaultshare value because _recalculateConnectedMarketsState calls creditDelegation.updateVaultLastDistributedValues and this function sets the lastvaultdistributedrealizeddebtusdpershare to the realizeddebtpervaultshare. This isnt the case and it returns 0 which is a bug
int128 lastvaultdistributedrealizeddebtusdpershare = marketMakingEngine.workaround_CreditDelegation_getlastVaultDistributedRealizedDebtUsdPerShare(fuzzVaultConfig.vaultId, fuzzMarketConfig.marketId);
console.log(lastvaultdistributedrealizeddebtusdpershare);
assert (realizeddebtpervaultshare != lastvaultdistributedrealizeddebtusdpershare);
//c on attempting to settle the vaults debt, creditdelegationbranch::settlevaultsdebt calls recalculatevaultcreditcapacity which should update the vaults debt but vault debt is never updated and although the vault should have debt from the creditdelegationbranch::depositcreditformarket call earlier, the value isnt updated and the vault debt will always be 0
vm.prank(address(perpsEngine));
marketMakingEngine.settleVaultsDebt(vaultIds);
int128 vaultdebt = marketMakingEngine.workaround_getVaultDebt(fuzzVaultConfig.vaultId);
console.log(vaultdebt);
assertEq(vaultdebt, 0);
}

OPTIONAL ADDON THAT MAY BE NEEDED IF RUNNING INTO WORKAROUND ERRORS WHEN RUNNING POC

Note that I added the following workarounds to VaultHarness.sol and MarketHarness.sol to get values I needed and I may have used them for POC's so if some of the tests do not work due to workaround functions not being found, add the following functions to VaultHarness.sol:

function workaround_CreditDelegation_getweight(uint128 vaultId, uint128 marketId) external view returns (uint128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.weight;
}
function workaround_Vault_getTotalCreditDelegationWeight(
uint128 vaultId
)
external view returns (uint128)
{
Vault.Data storage vaultData = Vault.load(vaultId);
return vaultData.totalCreditDelegationWeight ;
}
function workaround_CreditDelegation_getlastVaultDistributedRealizedDebtUsdPerShare(uint128 vaultId, uint128 marketId) external view returns (int128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.lastVaultDistributedRealizedDebtUsdPerShare;}
function workaround_CreditDelegation_setvalueUsd(uint128 vaultId, uint128 marketId, uint128 valueUsd) external {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
creditDelegation.valueUsd = valueUsd;
}
function workaround_CreditDelegation_getlastVaultDistributedUnrealizedDebtUsdPerShare(uint128 vaultId, uint128 marketId) external view returns (int128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.lastVaultDistributedUnrealizedDebtUsdPerShare;}
function workaround_CreditDelegation_getlastVaultDistributedUsdcCreditPerShare(uint128 vaultId, uint128 marketId) external view returns (uint128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.lastVaultDistributedUsdcCreditPerShare;}
function workaround_CreditDelegation_getlastVaultDistributedWethRewardPerShare(uint128 vaultId, uint128 marketId) external view returns (uint128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.lastVaultDistributedWethRewardPerShare;}
Add the following functions to MarketHarness.sol:
function workaround_gettotalWethReward(uint128 marketId) external view returns (uint256) {
Market.Data storage market = Market.load(marketId);
return market.wethRewardPerVaultShare;
}
function workaround_getrealizedDebtUsdPerVaultShare(uint128 marketId) external view returns (int128) {
Market.Data storage market = Market.load(marketId);
return market.realizedDebtUsdPerVaultShare;
}

After registering the selectors of these functions in TreeProxyUtils.sol and increasing the bytes array size, it should work as expected and return the correct values

Impact

Due to the guard clause preventing updates to the credit delegation branch's state, the vault's debt accumulation mechanism fails to capture any changes from connected markets. This results in:

Inaccurate Debt Accounting: The lastVaultDistributedRealizedDebtUsdPerShare remains at its initial zero value, causing the calculated debt changes to always be zero. As a consequence, the vault’s total debt never updates to reflect the actual market conditions.

Settlement Failures: When the system attempts to settle the vault’s debt (via functions such as settlevaultsdebt), it relies on these accumulated values. With the debt always recorded as zero, the settlement process will be flawed, potentially causing imbalances in user accounts and misallocations of funds.

Tools Used

Manual Review, Foundry

Recommendations

Remove the Guard Clause in Vault::recalculateVaultsCreditCapacity:
The root cause of the issue is the following code block:

if (
ctx.realizedDebtChangeUsdX18.isZero() && ctx.unrealizedDebtChangeUsdX18.isZero()
&& ctx.usdcCreditChangeX18.isZero() && ctx.wethRewardChangeX18.isZero()
) {
continue;
}

Removing this block will prevent the function from skipping the update to creditDelegation.updateVaultLastDistributedValues. This change will allow the per-share debt values (and other related metrics) to be updated correctly, even when the computed change appears to be zero on the first accumulation event.

In place of the removed guard clause, introduce logic that explicitly compares the current market debt values (as obtained by market.getRealizedDebtUsd() and market.getUnrealizedDebtUsd()) with their last distributed counterparts stored in the credit delegation. Update the credit delegation only if there is a meaningful change. For example, add a conditional check such as:

bool hasDebtChanged =
!market.getRealizedDebtUsd().eq(creditDelegation.lastVaultDistributedRealizedDebtUsdPerShare) ||
!market.getUnrealizedDebtUsd().eq(creditDelegation.lastVaultDistributedUnrealizedDebtUsdPerShare);
if (!hasDebtChanged) {
// No change detected, skip update.
continue;
}

This delta check must be implemented so that updates are only skipped when there is absolutely no change in the market’s debt values relative to the last distributed values, ensuring that even an initial update (where the values might be zero) is correctly recorded and also preventing the same debt being redistributed to vaults everytime Vault:recalculatevaultscreditcapacity is called.

Review Initialization of Last Distributed Values: Evaluate the initialization logic for lastVaultDistributedRealizedDebtUsdPerShare (and the corresponding variables for unrealized debt, USDC credit, and WETH reward). Ensure that the first accumulation event correctly captures and sets these values so that subsequent changes can be computed accurately.

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Vault and market state updates are incorrectly skipped when lastVaultDistributed values are zero, requiring WETH fees before accounting starts working

Support

FAQs

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