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

Due to not updating the Debt , the protocol will apply untended premium or discount

TheKhans

Summary

When market receive profit we call the depositCreditForMarket than we update the relevant collateral state , which will be applied to all the connected vaults of this market , in case of swapping of usd tokens with the vault assets. we apply premium/discount factor, depending on the state of vault either profit or lose.However the State of connected vaults were not updated in these calls which will effect the the discountPremiumFactor of vault. further details in next section.

Vulnerability Details

Here I focused on collateralAddr==usdc. when market receive funds we will call settleCreditDeposit :

/src/market-making/branches/CreditDelegationBranch.sol:181
181: function depositCreditForMarket(
182: uint128 marketId,
183: address collateralAddr,
184: uint256 amount
185: )
186: external
187: onlyRegisteredEngine(marketId)
188: {
189: if (amount == 0) revert Errors.ZeroInput("amount");
190:
191: // loads the collateral's data storage pointer, must be enabled
192: Collateral.Data storage collateral = Collateral.load(collateralAddr);
193: collateral.verifyIsEnabled();
194:
195: // loads the market's data storage pointer, must have delegated credit so
196: // engine is not depositing credit to an empty distribution (with 0 total shares)
197: // although this should never happen if the system functions properly.
198: Market.Data storage market = Market.loadLive(marketId);
199: if (market.getTotalDelegatedCreditUsd().isZero()) {
200: revert Errors.NoDelegatedCredit(marketId);
201: }
202:
203: // uint256 -> UD60x18 scaling decimals to zaros internal precision
204: UD60x18 amountX18 = collateral.convertTokenAmountToUd60x18(amount);
205:
206: // caches the usdToken address
207: address usdToken = MarketMakingEngineConfiguration.load().usdTokenOfEngine[msg.sender];
208:
209: // caches the usdc
210: address usdc = MarketMakingEngineConfiguration.load().usdc;
211: // note: storage updates must occur using zaros internal precision
212: if (collateralAddr == usdToken) {
213: // if the deposited collateral is USD Token, it reduces the market's realized debt
214: market.updateNetUsdTokenIssuance(unary(amountX18.intoSD59x18()));
215: } else {
216: if (collateralAddr == usdc) {
217: market.settleCreditDeposit(address(0), amountX18);
218: } else {
219: // deposits the received collateral to the market to be distributed to vaults
220: // to be settled in the future
221: market.depositCredit(collateralAddr, amountX18);
222: }
223: }
224: // transfers the margin collateral asset from the registered engine to the market making engine
225: // NOTE: The engine must approve the market making engine to transfer the margin collateral asset, see
226: // PerpsEngineConfigurationBranch::setMarketMakingEngineAllowance
227: // note: transfers must occur using token native precision
228: IERC20(collateralAddr).safeTransferFrom(msg.sender, address(this), amount);
229: // emit an event
230: emit LogDepositCreditForMarket(msg.sender, marketId, collateralAddr, amount);
231: }

in settleCreditDeposit function we update usdcCreditPerVaultShare and realizedDebtUsdPerVaultShare.

/src/market-making/leaves/Market.sol:444
444: function settleCreditDeposit(Data storage self, address settledAsset, UD60x18 netUsdcReceivedX18) internal {
445: // removes the credit deposit asset that has just been settled for usdc
446: self.creditDeposits.remove(settledAsset);
447:
448: // calculate the usdc that has been accumulated per usd of credit delegated to the market
449: UD60x18 addedUsdcPerCreditShareX18 = netUsdcReceivedX18.div(ud60x18(self.totalDelegatedCreditUsd));
450:
451: // add the usdc acquired to the accumulated usdc credit variable
452: self.usdcCreditPerVaultShare =
453: ud60x18(self.usdcCreditPerVaultShare).add(addedUsdcPerCreditShareX18).intoUint128();
454:
455: // deduct the amount of usdc credit from the realized debt per vault share, so we don't double count it
456: self.realizedDebtUsdPerVaultShare = sd59x18(self.realizedDebtUsdPerVaultShare).sub(
457: addedUsdcPerCreditShareX18.intoSD59x18()
458: ).intoInt256().toInt128();
459:
460: }

realizedDebtUsdPerVaultShare is important to find that the vault is in credit or debt.

/src/market-making/leaves/Vault.sol:278
278: function _recalculateConnectedMarketsState(
279: Data storage self,
280: uint128[] memory connectedMarketsIdsCache,
281: bool shouldRehydrateCache
282: )
283: private
284: returns (
285: uint128[] memory rehydratedConnectedMarketsIdsCache,
286: SD59x18 vaultTotalRealizedDebtChangeUsdX18,
287: SD59x18 vaultTotalUnrealizedDebtChangeUsdX18,
288: UD60x18 vaultTotalUsdcCreditChangeX18,
289: UD60x18 vaultTotalWethRewardChangeX18
290: )
291: {
...
330: // prevent division by zero
331: if (!market.getTotalDelegatedCreditUsd().isZero()) {
332: // get the vault's accumulated debt, credit and reward changes from the market to update its stored
333: // values
334: (
335: ctx.realizedDebtChangeUsdX18,
336: ctx.unrealizedDebtChangeUsdX18,
337: ctx.usdcCreditChangeX18,
338: ctx.wethRewardChangeX18
339: ) = market.getVaultAccumulatedValues(
340: ud60x18(creditDelegation.valueUsd),
341: sd59x18(creditDelegation.lastVaultDistributedRealizedDebtUsdPerShare),
342: sd59x18(creditDelegation.lastVaultDistributedUnrealizedDebtUsdPerShare),
343: ud60x18(creditDelegation.lastVaultDistributedUsdcCreditPerShare),
344: ud60x18(creditDelegation.lastVaultDistributedWethRewardPerShare)
345: );
346: }
...
350: if (
351: ctx.realizedDebtChangeUsdX18.isZero() && ctx.unrealizedDebtChangeUsdX18.isZero()
352: && ctx.usdcCreditChangeX18.isZero() && ctx.wethRewardChangeX18.isZero()
353: ) {
354: continue; // skip the iteration
355: }
356:
...
371: // update the last distributed debt, credit and reward values to the vault's credit delegation to the
372: // given market id, in order to keep next calculations consistent
373: creditDelegation.updateVaultLastDistributedValues(
374: sd59x18(market.realizedDebtUsdPerVaultShare),
375: sd59x18(market.unrealizedDebtUsdPerVaultShare),
376: ud60x18(market.usdcCreditPerVaultShare),
377: ud60x18(market.wethRewardPerVaultShare)
378: );
379: }
380: }

This function will update realizedDebtUsdPerVaultShare per vault.
To find the getAmountOfAssetOut we first get the getTotalDebt() of vault than we passed that debt to getPremiumDiscountFactor function.

/src/market-making/branches/StabilityBranch.sol:100
100: function getAmountOfAssetOut(
101: uint128 vaultId,
102: UD60x18 usdAmountInX18,
103: UD60x18 indexPriceX18
104: )
105: public
106: view
107: returns (UD60x18 amountOutX18)
108: {
109: // fetch the vault's storage pointer
110: Vault.Data storage vault = Vault.load(vaultId);
111:
112: // fetch the vault's total assets in USD; if the vault is empty
113: // revert here to prevent panic from subsequent divide by zero
114: UD60x18 vaultAssetsUsdX18 = ud60x18(IERC4626(vault.indexToken).totalAssets()).mul(indexPriceX18);
115: if (vaultAssetsUsdX18.isZero()) revert Errors.InsufficientVaultBalance(vaultId, 0, 0);
116:
117: // we use the vault's net sum of all debt types coming from its connected markets to determine the swap rate
118: SD59x18 vaultDebtUsdX18 = vault.getTotalDebt(); // this is zero but why ?
119:
120: // calculate the premium or discount that may be applied to the vault asset's index price
121: // note: if no premium or discount needs to be applied, the premiumDiscountFactorX18 will be
122: // 1e18 (UD60x18 one value)
123: UD60x18 premiumDiscountFactorX18 =
124: UsdTokenSwapConfig.load().getPremiumDiscountFactor(vaultAssetsUsdX18, vaultDebtUsdX18);
125:
126: // get amounts out taking into consideration the CL price and the premium/discount
127: amountOutX18 = usdAmountInX18.div(indexPriceX18).mul(premiumDiscountFactorX18);
130:
131: }

getTotalDebt will return the debt of a vault on basis of this than we will either apply discount or premium.

/src/market-making/leaves/Vault.sol:227
227: function getTotalDebt(Data storage self) internal view returns (SD59x18 totalDebtUsdX18) {
228: totalDebtUsdX18 = getUnsettledRealizedDebt(self).add(sd59x18(self.marketsUnrealizedDebtUsd));
229: }
....
241: function getUnsettledRealizedDebt(Data storage self)
242: internal
243: view
244: returns (SD59x18 unsettledRealizedDebtUsdX18)
245: {
246: unsettledRealizedDebtUsdX18 =
247: sd59x18(self.marketsRealizedDebtUsd).add(unary(ud60x18(self.depositedUsdc).intoSD59x18()));
248: }

So now lets have a look on how we calculate the getPremiumDiscountFactor.

function getPremiumDiscountFactor(
Data storage self,
UD60x18 vaultAssetsValueUsdX18,
SD59x18 vaultDebtUsdX18
)
internal
view
returns (UD60x18 premiumDiscountFactorX18)
{
// calculate the vault's tvl / debt absolute value, positive means we'll apply a discount, negative means
// we'll apply a premium
​
// console.log("vaultDebtUsdX18--------++++++++++++++++++++",vaultDebtUsdX18.intoUint256(),vaultAssetsValueUsdX18.intoUint256());
UD60x18 vaultDebtTvlRatioAbs = vaultDebtUsdX18.abs().intoUD60x18().div(vaultAssetsValueUsdX18);
​
// cache the minimum x value of the premium / discount curve
UD60x18 pdCurveXMinX18 = ud60x18(0.1e18);//ud60x18(self.pdCurveXMin);
// cache the maximum x value of the premium / discount curve
UD60x18 pdCurveXMaxX18 =ud60x18(0.8e18); //ud60x18(self.pdCurveXMax);
​
// if the vault's debt / tvl ratio is less than or equal to the minimum x value of the premium / discount
// curve, then we don't apply any premium or discount
if (vaultDebtTvlRatioAbs.lte(pdCurveXMinX18)) {
premiumDiscountFactorX18 = UD60x18_UNIT;
return premiumDiscountFactorX18;
}
​
// if the vault's debt / tvl ratio is greater than or equal to the maximum x value of the premium / discount
// curve, we use the max X value, otherwise, use the calculated vault tvl / debt ratio
UD60x18 pdCurveXX18 = vaultDebtTvlRatioAbs.gte(pdCurveXMaxX18) ? pdCurveXMaxX18 : vaultDebtTvlRatioAbs;
​
// cache the minimum y value of the premium / discount curve
UD60x18 pdCurveYMinX18 = ud60x18(0.3e18);//ud60x18(self.pdCurveYMin);
// cache the maximum y value of the premium / discount curve
UD60x18 pdCurveYMaxX18 = ud60x18(0.9e18);//ud60x18(self.pdCurveYMax);
​
// cache the exponent that determines the steepness of the premium / discount curve
UD60x18 pdCurveZX18 = ud60x18(0.6e18);//ud60x18(self.pdCurveZ);
// calculate the y point of the premium or discount curve given the x point
UD60x18 pdCurveYX18 = pdCurveYMinX18.add(
pdCurveYMaxX18.sub(pdCurveYMinX18).mul(
pdCurveXX18.sub(pdCurveXMinX18).div(pdCurveXMaxX18.sub(pdCurveXMinX18)).pow(pdCurveZX18)
)
);
// if the vault is in credit, we apply a discount, otherwise, we apply a premium
premiumDiscountFactorX18 =
vaultDebtUsdX18.lt(SD59x18_ZERO) ? UD60x18_UNIT.sub(pdCurveYX18) : UD60x18_UNIT.add(pdCurveYX18);
}

Note Here I have assigned dummy data to pdCurveXMinX18 , pdCurveXMaxX18 , pdCurveYMinX18 , pdCurveYMaxX18 and pdCurveZX18 because if not it will revert which is separate issue i have submitted.

So now lets just wrap it:
Assume the vault is in +debt , we deposit USDC to the market which will give us new value of self.realizedDebtUsdPerVaultShare and self.usdcCreditPerVaultShare which we were suppose to apply to the all vaults connect with this market but we are not doing it for impact see the impact section of report. The only calls to the function updateVaultCreditCapacity will not fix this because the lastVaultDistributedRealizedDebtUsdPerShareX18 will be zero and no changes would be applied to the vault.

/src/market-making/leaves/Market.sol:303
303: realizedDebtChangeUsdX18 = !lastVaultDistributedRealizedDebtUsdPerShareX18.isZero()
304: ?sd59x18(self.realizedDebtUsdPerVaultShare).sub(lastVaultDistributedRealizedDebtUsdPerShareX18).mul(vaultCreditShareX18.intoSD59x18())
305: : SD59x18_ZERO;
306:

POC

I have assume few things for POC :

  1. Add dummy value to the function getPremiumDiscountFactor you can copy it from above section.

  2. Called the receiveMarketFee function so that subsequent call to updateVaultCreditCapacity will apply latest changes to the vault.

function test_depositCreditForMarket_state_no_update(
)
external
whenCallerIsKeeper
whenRequestWasNotYetProcessed
whenSwapRequestNotExpired
{
uint256 vaultId = 100;
uint256 marketId = 100;
uint256 vaultAssetsBalance = 5000e16;
uint256 swapAmount = 1000e6;
uint128 vaultDebtAbsUsd = uint128(2e18);
bool useCredit= false;
TestFuzz_WhenSlippageCheckPassesAndThePremiumOrDiscountIsNotZero_Context memory ctx;
​
ctx.fuzzVaultConfig = getFuzzVaultConfig(vaultId);
​
ctx.oneAsset = 10 ** ctx.fuzzVaultConfig.decimals;
​
changePrank({ msgSender: users.owner.account });
marketMakingEngine.configureUsdTokenSwapConfig(1, 30, type(uint96).max);
marketMakingEngine.workaround_setVaultDebt(uint128(ctx.fuzzVaultConfig.vaultId),int128(vaultDebtAbsUsd));
marketMakingEngine.workaround_setVaultDepositedUsdc(uint128(ctx.fuzzVaultConfig.vaultId),0);
marketMakingEngine.configureEngine(address(marketMakingEngine), address(newUSDToken), true);
​
changePrank({ msgSender: users.naruto.account });
​
// bound the vault assets balance to be between 1 asset unit and the deposit cap
// vaultAssetsBalance = bound({ x: vaultAssetsBalance, min: ctx.oneAsset, max: ctx.fuzzVaultConfig.depositCap });
​
// bound the vault's total credit or debt
// vaultDebtAbsUsd = bound({ x: vaultDebtAbsUsd, min: ctx.oneAsset / 2, max: vaultAssetsBalance });
console.log("----------vaultDebtAbsUsd-----------",vaultDebtAbsUsd);
deal({
token: address(ctx.fuzzVaultConfig.asset),
to: ctx.fuzzVaultConfig.indexToken,
give: vaultAssetsBalance
});
​
swapAmount = vaultAssetsBalance;
console.log("---------------swapAmount---------------",swapAmount);
​
deal({ token: address(usdToken), to: users.naruto.account, give: swapAmount });
​
ctx.fuzzPerpMarketCreditConfig = getFuzzPerpMarketCreditConfig(marketId);
ctx.engine = IMockEngine(perpMarketsCreditConfig[ctx.fuzzPerpMarketCreditConfig.marketId].engine);
ctx.minAmountOut = 0;
UD60x18 priceUsdX18 = IPriceAdapter(vaultsConfig[ctx.fuzzVaultConfig.vaultId].priceAdapter).getPrice();
​
ctx.priceData = getMockedSignedReport(ctx.fuzzVaultConfig.streamId, priceUsdX18.intoUint256());
ctx.usdTokenSwapKeeper = usdTokenSwapKeepers[ctx.fuzzVaultConfig.asset];
deal({ token: address(usdc), to: address(ctx.engine), give: 10_000e30});
changePrank({ msgSender: address(ctx.engine) });
uint256 marketFees = 1_000_000_000_000_000_000;
IERC20 wethFeeToken = IERC20(getFuzzVaultConfig(WETH_CORE_VAULT_ID).asset);
deal({ token: address(wethFeeToken), to: address(ctx.engine), give: marketFees });
​
changePrank({ msgSender: address((ctx.engine)) });
​
marketMakingEngine.depositCreditForMarket(ctx.fuzzPerpMarketCreditConfig.marketId,address(usdc),1_000_000_000_000_000);
marketMakingEngine.receiveMarketFee(ctx.fuzzPerpMarketCreditConfig.marketId, address(wethFeeToken), marketFees);//@audit added just to proof that if state update occur the received assets will be different
marketMakingEngine.depositCreditForMarket(ctx.fuzzPerpMarketCreditConfig.marketId,address(usdc),1_000_000_000_000_000_00);
UD60x18 positveDebtAmountOut =
marketMakingEngine.getAmountOfAssetOut(ctx.fuzzVaultConfig.vaultId, ud60x18(swapAmount), priceUsdX18);
​
// marketMakingEngine.updateVaultCreditCapacity(ctx.fuzzVaultConfig.vaultId);// uncomment it to see the revert if the update applies it will work as expected
UD60x18 negativeDebtAmountOut =
marketMakingEngine.getAmountOfAssetOut(ctx.fuzzVaultConfig.vaultId, ud60x18(swapAmount), priceUsdX18);
console.log("+veCase" , positveDebtAmountOut.intoUint256());
console.log("-veCase" , negativeDebtAmountOut.intoUint256());
assertEq(positveDebtAmountOut.intoUint256() , negativeDebtAmountOut.intoUint256()); // @audit it still gives discount instead of charging premium
}

run with command : forge test --mt test_depositCreditForMarket_state -vvvv.
In the above case, both values are equal, indicating that the vault still applies a discount even though it is supposed to charge a premium. To test other scenarios, uncomment updateVaultCreditCapacity.

Impact

The vault will still give discount or charge premium due not not updating the vault debt state. it will result in lose for vault in case the vault is in credit or vice versa.

Tools Used

Manual Review

Recommendations

The Fix for the issue lies in these two functions :

  1. getVaultAccumulatedValues

  2. depositCreditForMarket
    For getVaultAccumulatedValues following fix is recommended:

diff --git a/src/market-making/leaves/Market.sol b/src/market-making/leaves/Market.sol
index d094374..aab3bb5 100644
--- a/src/market-making/leaves/Market.sol
+++ b/src/market-making/leaves/Market.sol
@@ -299,11 +299,9 @@ library Market {
// 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
// console.log("lastVaultDistributedRealizedDebtUsdPerShareX18------------",lastVaultDistributedRealizedDebtUsdPerShareX18.intoInt256());
- realizedDebtChangeUsdX18 = !lastVaultDistributedRealizedDebtUsdPerShareX18.isZero()
- ? realizedDebtChangeUsdX18
- : SD59x18_ZERO;
+ // realizedDebtChangeUsdX18 = ;
+ realizedDebtChangeUsdX18 = sd59x18(self.realizedDebtUsdPerVaultShare).sub(lastVaultDistributedRealizedDebtUsdPerShareX18).mul(vaultCreditShareX18.intoSD59x18());
+ // : SD59x18_ZERO;

Add the Following changes to CreditDelegationBranch

diff --git a/src/market-making/branches/CreditDelegationBranch.sol b/src/market-making/branches/CreditDelegationBranch.sol
index c3e04fa..9cefa39 100644
--- a/src/market-making/branches/CreditDelegationBranch.sol
+++ b/src/market-making/branches/CreditDelegationBranch.sol
@@ -208,11 +208,9 @@ contract CreditDelegationBranch is EngineAccessControl {
// caches the usdc
address usdc = MarketMakingEngineConfiguration.load().usdc;
// note: storage updates must occur using zaros internal precision
if (collateralAddr == usdToken) {
// if the deposited collateral is USD Token, it reduces the market's realized debt
market.updateNetUsdTokenIssuance(unary(amountX18.intoSD59x18())); // @note -amountX18
} else {
@@ -229,6 +227,9 @@ contract CreditDelegationBranch is EngineAccessControl {
// PerpsEngineConfigurationBranch::setMarketMakingEngineAllowance
// note: transfers must occur using token native precision
IERC20(collateralAddr).safeTransferFrom(msg.sender, address(this), amount);
+
+ Vault.recalculateVaultsCreditCapacity(market.getConnectedVaultsIds());
+
// emit an event
emit LogDepositCreditForMarket(msg.sender, marketId, collateralAddr, amount);
}
Updates

Lead Judging Commences

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

Vault::_recalculateConnectedMarketsState doesn't update market's realized debt per vault share after clearing market's debt, remove the zero check

Support

FAQs

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