Unpredictable behavior from no range Checks for Wrapped UFixed6 Values, If a UFixed6 value representing a percentage exceeds 100%, it leads to incorrect calculations, fees, or other critical parameters

Summary

The code lacks range checks for UFixed6 values extracted from the bit fields. This vulnerability can lead to values exceeding their intended ranges, potentially causing unpredictable behavior and vulnerabilities in the protocol.

Vulnerability Detail

In the code provided, there is a missing range-check mechanism for UFixed6 values that are extracted from the bit fields. These UFixed6 values represent fixed-point numbers with a specified number of decimal places. It is essential to ensure that these extracted values do not exceed their intended ranges to maintain the protocol's stability and integrity.

Impact

Values that exceed their intended ranges can have various adverse effects on the protocol. For example, if a UFixed6 value representing a percentage exceeds 100%, it may lead to incorrect calculations, fees, or other critical parameters. This can result in unexpected behavior and vulnerabilities within the protocol.

Code Snippet

code snippet from the validate function where range checks should be applied to ensure that the extracted UFixed6 values remain within their intended ranges:

function validate(RiskParameter memory self, ProtocolParameter memory protocolParameter) public pure {
    if (
        self.takerFee.max(self.takerSkewFee).max(self.takerImpactFee).max(self.makerFee).max(self.makerImpactFee)
        .gt(protocolParameter.maxFee)
    ) revert RiskParameterStorageInvalidError();

    if (
        self.minLiquidationFee.max(self.maxLiquidationFee).max(self.minMargin).max(self.minMaintenance)
        .gt(protocolParameter.maxFeeAbsolute)
    ) revert RiskParameterStorageInvalidError();

    if (self.liquidationFee.gt(protocolParameter.maxCut)) revert RiskParameterStorageInvalidError();

    if (
        self.utilizationCurve.minRate.max(self.utilizationCurve.maxRate).max(self.utilizationCurve.targetRate).max(self.pController.max)
        .gt(protocolParameter.maxRate)
    ) revert RiskParameterStorageInvalidError();

    // ... (other validation checks)
}

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial/contracts/types/RiskParameter.sol#L142

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial/contracts/types/RiskParameter.sol#L144

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial/contracts/types/RiskParameter.sol#L149

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial/contracts/types/RiskParameter.sol#L156

Tool used

Manual Review

Recommendation

Implement range checks for UFixed6 values extracted from the bit fields to ensure they stay within their intended ranges. Here is an example of a recommended mitigation step:

function validate(RiskParameter memory self, ProtocolParameter memory protocolParameter) public pure {
    // ... (other validation checks)

    // Ensure that UFixed6 values do not exceed their intended ranges
    if (self.takerFee.gt(UFixed6Lib.ONE)) revert RiskParameterStorageInvalidError();
    if (self.takerSkewFee.gt(UFixed6Lib.ONE)) revert RiskParameterStorageInvalidError();
    if (self.takerImpactFee.gt(UFixed6Lib.ONE)) revert RiskParameterStorageInvalidError();
    if (self.makerFee.gt(UFixed6Lib.ONE)) revert RiskParameterStorageInvalidError();
    if (self.makerImpactFee.gt(UFixed6Lib.ONE)) revert RiskParameterStorageInvalidError();

    // ... (other range checks)
}

In the recommended mitigation step, we ensure that each UFixed6 value is checked against its intended range (in this case, not greater than 100%). If any value exceeds the range, the contract reverts with an error to prevent incorrect data from being processed. Repeat this pattern for all relevant UFixed6 values in the code.

sherlock-audit / 2023-09-perennial-judging

0xVinylDavyl - Unpredictable behavior from no range Checks for Wrapped UFixed6 Values, If a UFixed6 value representing a percentage exceeds 100%, it leads to incorrect calculations, fees, or other critical parameters #22