Part 2

Summary

The CreditDelegation.updateVaultLastDistributedValues() function inconsistently uses signed (SD59x18) and unsigned (UD60x18) types when updating the state variables of the CreditDelegation.Data struct. Specifically, signed values are converted to int128, while unsigned values are converted to uint128. This inconsistency could lead to precision loss, unexpected behavior, or overflow/underflow during type conversions.

Vulnerability Details

Code Analysis

The updateVaultLastDistributedValues() function is defined as follows:

Key Observations:

1. Signed vs. Unsigned Types :

   • The function accepts two signed (SD59x18) and two unsigned (UD60x18) normalized values.

   • These values are converted to int128 and uint128, respectively, before being stored in the CreditDelegation.Data struct.

2. Type Conversion Risks :

   • Converting SD59x18 to int128 could result in precision loss if the value exceeds the range of int128.

   • Similarly, converting UD60x18 to uint128 could cause overflow if the value exceeds the range of uint128.

3. Inconsistent Handling :

   • The use of both signed and unsigned types introduces complexity and increases the risk of errors during calculations or comparisons.

Attack Scenario

1. An attacker or user provides extreme values for vaultDistributedRealizedDebtUsdPerShareX18 or vaultDistributedUsdcCreditPerShareX18.

2. If the provided SD59x18 value exceeds the range of int128 or the UD60x18 value exceeds the range of uint128, the conversion will fail or produce incorrect results.

3. Incorrect state updates could lead to accounting discrepancies, affecting debt and reward distributions.

Impact

• Precision Loss : Converting SD59x18 to int128 could truncate significant digits, leading to inaccurate debt or reward calculations.

• Overflow/Underflow : Converting UD60x18 to uint128 could cause overflow, resulting in incorrect or corrupted state data.

• Accounting Errors : Inconsistent handling of signed and unsigned types could propagate errors to other parts of the system, affecting users and the protocol.

Tools Used

1. Manual Code Review : Analyzed the type conversions and their potential risks in the updateVaultLastDistributedValues() function.

2. Slither : Static analysis tool used to identify unsafe type conversions and potential overflow/underflow risks.

3. MythX : Security analysis platform used to verify vulnerabilities related to signed/unsigned type mismatches.

Recommendations

Short-Term Fix

Add explicit checks to ensure that the provided values fit within the target type's range before performing conversions. For example:

Long-Term Fix

1. Consistent Type Usage : Standardize the use of signed or unsigned types throughout the library to reduce complexity and minimize risks.

   • For example, use UD60x18 consistently for all monetary values since they are inherently non-negative.

2. Safe Math Libraries : Use libraries like OpenZeppelin's SafeCast to perform safe type conversions with built-in overflow/underflow checks.

3. Event Logging : Emit events whenever state variables are updated to provide transparency and enable monitoring.

   event VaultLastDistributedValuesUpdated(

   uint128 vaultId,

   uint128 marketId,

   int128 realizedDebt,

   int128 unrealizedDebt,

   uint128 usdcCredit,

   uint128 wethReward

   );

   ​

   function updateVaultLastDistributedValues(...) internal {

   // Perform updates...

   emit VaultLastDistributedValuesUpdated(

   self.vaultId,

   self.marketId,

   self.lastVaultDistributedRealizedDebtUsdPerShare,

   self.lastVaultDistributedUnrealizedDebtUsdPerShare,

   self.lastVaultDistributedUsdcCreditPerShare,

   self.lastVaultDistributedWethRewardPerShare

   );

   }