Incorrect Calculation in _calculateTemporaryReserveRatioBips Function

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/access/MarketConstraintHooks.sol#L160

Vulnerability details

Issue

The _calculateTemporaryReserveRatioBips function in the MarketConstraintHooks contract is designed to calculate a temporary reserve ratio when the annual interest rate is reduced.

The function incorrectly calculates the temporary reserve ratio. It aims to double the relative difference between the old and new APRs but applies the min function incorrectly, leading to a capped temporary reserve ratio instead of a potentially doubled one.

The function aims to incentivize borrowers to avoid drastically reducing the APR by temporarily increasing the reserve ratio based on the reduction amount. This mechanism protects lenders from drastic yield decreases.

Code Snippet

function _calculateTemporaryReserveRatioBips(
    uint256 annualInterestBips,
    uint256 originalAnnualInterestBips,
    uint256 originalReserveRatioBips
) internal pure returns (uint16 temporaryReserveRatioBips) {
    // Calculate the relative reduction in the interest rate in bips,
    // bound to a maximum of 100%
    uint256 relativeDiff = MathUtils.mulDiv(
        10000,
        originalAnnualInterestBips - annualInterestBips,
        originalAnnualInterestBips
    );

    // If the reduction is 25% (2500 bips) or less, return the original reserve ratio
    if (relativeDiff <= 2500) {
        temporaryReserveRatioBips = uint16(originalReserveRatioBips);
    } else {
        // Calculate double the relative reduction in the interest rate in bips,
        // bound to a maximum of 100%
        uint256 boundRelativeDiff = MathUtils.min(10000, 2 * relativeDiff);

        // If the bound relative diff is lower than the existing reserve ratio, return the latter.
        temporaryReserveRatioBips = uint16(
            MathUtils.max(boundRelativeDiff, originalReserveRatioBips)
        );
    }
}

Impact

The incorrect use of MathUtils.max() in the calculation prevents the temporary reserve ratio from exceeding 100%, even when the relative difference is doubled. This undermines the intended disincentive for large APR reductions as the temporary reserve ratio does not scale as intended.

Scenario

Consider an original APR of 8% (800 bips) and an original reserve ratio of 10%. If the borrower reduces the APR to 1% (100 bips), the relative difference is 87.5%. Doubling this results in 175%, which should be the temporary reserve ratio. However, the code incorrectly limits it to 100%.

Fix

Replace MathUtils.max(boundRelativeDiff, originalReserveRatioBips) with just boundRelativeDiff:

temporaryReserveRatioBips = uint16(boundRelativeDiff);

Assessed type

Math

[laurenceday]

This finding is fairly comical and frustrating to see in equal measure, because the Wildcat V1 whitepaper specifies that the doubled cap is 100%, but I removed this from the Gitbook while rewriting it for v2 because it seemed so unnecessary to highlight that the cap ends at full collateralisation.

For the avoidance of doubt, from the whitepaper:

This mechanism has slightly changed (now changes below 25% relative difference don't trigger this requirement), but otherwise it is unchanged in V2.

More bluntly: be realistic. Even market termination only requires 100% collateralisation of the market: it's barking mad to insist that changing an APR should require anything more than that.

[3docSec]

As per sponsor's comment, hard to argue this is not intended behavior

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid