Part 2

Description

StabilityBranch::getAmountOfAssetOut will call UsdTokenSwapConfig::getPremiumDiscountFactor with the arguments UD60x18 vaultAssetsUsdX18 and SD59x18 vaultDebtUsdX18 the issue arises in the use of totalAssets() from the ERC-4626 standards.
totalAssets() returns a value of an asset via balanceOf(address(this)). This behavior directly influences UD60x18 vaultDebtTvlRatioAbs in UsdTokenSwapConfig::getPremiumDiscountFactor to return wrong values and therefor over- underpay users.

Vulnerable Code

StabilityBranch::getAmountOfAssetOut

Under 1. we fetch UD60x18 vaultAssetsUsdX18 and in 2. we pass it into UsdTokenSwapConfig::getPremiumDiscountFactor. Note the usage of totalAssets() which is manipulatable by donation attacks.

In UsdTokenSwapConfig::getPremiumDiscountFactor now:

As you can see here in 3. we are calculating vaultDebtTvlRatioAbs by taking the absolute of vaultDebtUsdX18 and dividing it with vaultAssetsValueUsdX18, which is our manipulatable value.
Under 4. and 5. we see the clamping mechanisms to keep pdCurveXX18 (or x in f(x) = y_min + Δy * ((x - x_min) / (x_max - x_min))^z) within the boundaries for x ∈ [x_min, x_max].
Within step 6. we can see the finalization of the premiumDiscountFactorX18 if our values passed the clamping mechanisms.

Now let's evaluate resulting scenarios

Scenario 1:

Let's say the vault carries a total assetValue of 1000 USD and would have a vaultDebt of -600 USD.
Also we are working with proposed inital parameters of x_min is 0.3 and x_max is 0.8 like in
/// @dev The proposed initial curve is defined as:
/// f(x) = 1 + 9 * ((x - 0.3) / 0.5)^3

following 3. we would assume UD60x18 vaultDebtTvlRatioAbs to be 600/1000 = 0.6 with a negative vaultDebtUsdX18 resulting in pdCurveYX18 = 2.944. Fast forward into 6. we would land premiumDiscountFactorX18 = -1.944.
Since, in this case, in reality x=0.6 and is therefor holding x ∈ [x_min, x_max] this would be the expected result of the calculation, however if someone were to donate 1000 USD value worth of asset into the vault before this transaction this calculation changes into the following:
600/2000 = 0.3 which, if we look into the code above under 4. would be clamped into premiumDiscountFactorX18 = 1 since x <= x_min. So in this scenario an returns an incorrect value.

Scenario 2:

Let's now say the vault carries a total assetValue of 1000 USD and would have a vaultDebt of 900 USD.
This would result in
UD60x18 vaultDebtTvlRatioAbs = 0.9 which would be clamped by 5. in above code to 0.8 by default and result in pdCurveYX18 = 10 and lastly result in premiumDiscountFactorX18 = 11.

If someone though were to donate now 9000 USD into the vault UD60x18 vaultDebtTvlRatioAbs = 0.9 resulting in premiumDiscountFactorX18 = 1 clamped via 4..

Impact

For the impact analysis it is worth mentioning that the protocol neither has a way to remove those funds from the vault nor any possibility to manage them as dead shares as they are. Depending on the position the vault is in at a point in time, donating into the vault can have different effects. Depending if the vaultDebt at the time of donation is positive or negative, users would either be able to withdraw way more than what they are supposed to withdraw and therefor directly affecting the solvency of the protocol or potentially getting underpaid, locking remaining funds in the vault for accumulation.
Also even without long term damages, short term manipulation of the return values can pose thread to the integrity of user funds or the solvency of the protocol as described above via MEV. The fact that the protocol has no way to deal with funds like that in the long term only amplifies the severity.

Likelihood: Low - Medium

Impact: High

Due to direct financial losses to the protocol and/or users I rate this as high.

Tools Used

Manual Review

Recommended Mitigation

One way to fix this is to include an internal accounting system and disconnect from the totalAssets() function.

Alternatively it would be possible to make "dead funds" usable by integrating a withdrawal or including them into the accounting as donations to users/the protocol.