Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong calculation in the Vault.sol's recalculateVaultsCreditCapacity's `_updateCreditDelegations` will most likely result in whole tx being reverted

hard1k

Summary

The Vault.sol's recalculateVaultsCreditCapacity's _updateCreditDelegationsuses wrong values to calculate the Vault's new updated credit delegation.

Vulnerability Details

The recalculateVaultsCreditCapacity function is called or used in mostly all of the protocol's function. Hence this is the core function of the codebase. What this function does is perform many operations like updating states, etc in regards to vaults and the connected markets. And one of these operations is that it updates the vault's credit delegation to its connected markets and this is done via _updateCreditDelegations function.
Now the ISSUE originates from this function. What is does that caches the connectedMarketIds connected to a vault and iterates through them and then takes previousCreditDelegationUsdX18 of vault to the market id and the totalCreditDelegationWeightCache i.e "The total amount of credit delegation weight in the vault." along with these creditDelegationShareX18 is taken which is computes as:- UD60x18 creditDelegationShareX18 = ud60x18(creditDelegation.weight).div(ud60x18(totalCreditDelegationWeightCache)); , vaultCreditCapacityUsdX18 i.e totalAssets - totalDebt . And after that by taking vaultCreditCapacity and multipllying it by creditDelegationShareX18 newCreditDelegationUsdX18 is computed. Lastly creditDeltaUsdX18 variable is computed and updateTotalDelegatedCredit function is called passing in this creditDelta param and in this function the the totalDelgatedCreditUsd is updated by adding the creditDelta to totalDelegatedCreditUsd. Now the issue here is in the calculation of the creditDeltaUsdX18 variable, this is of type UD60x18 i.e unsigned. And this is computed like this - UD60x18 creditDeltaUsdX18 = newCreditDelegationUsdX18.sub(previousCreditDelegationUsdX18); Now this will return a negative value if the previous delegation > new delegation. I'll explain this by giving the example calculation using some values.

Lets assume that for a market id and a vault:

previousCreditDelegationUsdX18 = 500000e18
totalCreditDelegationWeightCache = 1 million assets in 18 dec i.e zaros internal precision
creditDelegationShareX18 = creditDelegation.weight / totalCreditDelegationWeightCache = 1e18 as weight set previously in the updateVaultAndCreditDelegationWeight function.
vaultCreditCapacityUsdX18- this is totalAssets - totalDebt, so lets say total debt is 80ke18 that means 1 million - 100k = 900ke18
newCreditDelegationUsdX18 = vaultCreditCapacityUsdX18 * creditDelegationShareX18 i.e 50ke18 * 1e18 = 900ke18
Now the creditDelta is computed as- newCreditDelegationUsdX18.sub(previousCreditDelegationUsdX18); i.e 900ke18 - 1 million This will oviously return a negative value but this will revert as the creditDeltaUsdX18 is set as UD60x18 type. this will result in revert of the recalculateVaultsCreditCapacity function.

Lines of Code:
```
function _updateCreditDelegations(
Data storage self,
uint128[] memory connectedMarketsIdsCache,
bool shouldRehydrateCache
)
private
returns (uint128[] memory rehydratedConnectedMarketsIdsCache, SD59x18 vaultCreditCapacityUsdX18)
{
rehydratedConnectedMarketsIdsCache = new uint128[](connectedMarketsIdsCache.length);
// cache the vault id
uint128 vaultId = self.id;
​
// cache the connected markets length
uint256 connectedMarketsConfigLength = self.connectedMarkets.length;
​
// loads the connected markets storage pointer by taking the last configured market ids uint set
EnumerableSet.UintSet storage connectedMarkets = self.connectedMarkets[connectedMarketsConfigLength - 1];
​
// loop over each connected market id that has been cached once again in order to update this vault's
// credit delegations
for (uint256 i; i < connectedMarketsIdsCache.length; i++) {
// rehydrate the markets ids cache if needed
if (shouldRehydrateCache) {
rehydratedConnectedMarketsIdsCache[i] = connectedMarkets.at(i).toUint128();
} else {
rehydratedConnectedMarketsIdsCache[i] = connectedMarketsIdsCache[i];
}
​
// loads the memory cached market id
uint128 connectedMarketId = rehydratedConnectedMarketsIdsCache[i];
​
// load the credit delegation to the given market id
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, connectedMarketId);
​
// cache the previous credit delegation value
UD60x18 previousCreditDelegationUsdX18 = ud60x18(creditDelegation.valueUsd);
​
// cache the latest credit delegation share of the vault's credit capacity
uint128 totalCreditDelegationWeightCache = self.totalCreditDelegationWeight;
​
if (totalCreditDelegationWeightCache != 0) {
// get the latest credit delegation share of the vault's credit capacity
UD60x18 creditDelegationShareX18 =
ud60x18(creditDelegation.weight).div(ud60x18(totalCreditDelegationWeightCache));
​
// stores the vault's total credit capacity to be returned
vaultCreditCapacityUsdX18 = getTotalCreditCapacityUsd(self);
​
// if the vault's credit capacity went to zero or below, we set its credit delegation to that market
// to zero
UD60x18 newCreditDelegationUsdX18 = vaultCreditCapacityUsdX18.gt(SD59x18_ZERO)
? vaultCreditCapacityUsdX18.intoUD60x18().mul(creditDelegationShareX18)
: UD60x18_ZERO;
​
// calculate the delta applied to the market's total delegated credit
-> UD60x18 creditDeltaUsdX18 = newCreditDelegationUsdX18.sub(previousCreditDelegationUsdX18);
​
// loads the market's storage pointer and update total delegated credit
Market.Data storage market = Market.load(connectedMarketId);
-> market.updateTotalDelegatedCredit(creditDeltaUsdX18);
​
// if new credit delegation is zero, we clear the credit delegation storage
if (newCreditDelegationUsdX18.isZero()) {
creditDelegation.clear();
} else {
// update the credit delegation stored usd value
creditDelegation.valueUsd = newCreditDelegationUsdX18.intoUint128();
}
}
}
}```

Impact

Due to the wrong accounting of the creditDelta variable the recalculateVaultsCreditCapacity will revert which is one of most important and core functionality of the system

Recommendation

Either change the creditDeltaUsdX18 from type SD60x18 to UD60x18 or change the computation of it to the inverse of its current. for eg:

- UD60x18 creditDeltaUsdX18 = newCreditDelegationUsdX18.sub(previousCreditDelegationUsdX18);
+ UD60x18 creditDeltaUsdX18 = previousCreditDelegationUsdX18.sub(newCreditDelegationUsdX18);
Updates

Lead Judging Commences

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

Vault::_updateCreditDelegations uses unsigned UD60x18 for credit delegation delta calculation which will underflow on any decrease in credit delegation amount

Support

FAQs

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

Give us feedback!