code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Unsafe casting of Chainlink data feed answers #164

Closed c4-bot-1 closed 5 months ago

c4-bot-1 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/DerivativeOracles/PtOraclePure.sol#L103 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/DerivativeOracles/PendleLpOracle.sol#L118 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/DerivativeOracles/PtOracleDerivative.sol#L122-L126

Vulnerability details

Impact

Unsafe casting is performed in PendleLpOracle, PtOraclePure, PtOracleDerivative. Casting from int256 to uint256 when a price is negative will lead to underflow and return an unwanted result.

Proof of Concept

Contract functions consuming Chainlink oracles in PendleLpOracle, PtOraclePure, and PtOracleDerivative perform an unsafe casting from int256 to uint256.

function latestAnswer()
    public
    view
    returns (uint256)
{
    (
        ,
        int256 answerFeed,
        ,
        ,
    ) = FEED_ASSET.latestRoundData();

    (
        bool increaseCardinalityRequired,
        ,
        bool oldestObservationSatisfied
    ) = ORACLE_PENDLE_PT.getOracleState(
        PENDLE_MARKET_ADDRESS,
        TWAP_DURATION
    );

    if (increaseCardinalityRequired == true) {
        revert CardinalityNotSatisfied();
    }

    if (oldestObservationSatisfied == false) {
        revert OldestObservationNotSatisfied();
    }

    uint256 lpRate = _getLpToAssetRateWrapper(
        IPMarket(PENDLE_MARKET_ADDRESS),
        TWAP_DURATION
    );

    return lpRate
        * uint256(answerFeed)
        / PRECISION_FACTOR_E18;
}

The cast assumes that the price consistently remains positive. This might not be the case if the oracle has malfunctioned or a market crash has occurred.

Tools Used

Manual Review

Recommended Mitigation Steps

Always check if the price that's int256 is greater than 0 before casting to uint256

Assessed type

Oracle

GalloDaSballo commented 6 months ago

Prob best as QA since we know that negative is only for Commodities whereas aggregators for crypto have min and max above 0

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as primary issue

c4-judge commented 5 months ago

trust1995 changed the severity to QA (Quality Assurance)

c4-judge commented 5 months ago

trust1995 marked the issue as grade-c