code-423n4 / 2022-11-paraspace-findings

7 stars 4 forks source link

Upgraded Q -> M from #472 [1674665995647] #519

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #472 as M risk. The relevant finding follows:

[L-01] CHAINLINK AGGREGATOR IS NOT SUFFICIENTLY VALIDATED AND CAN RETURN STALE ANSWER As shown below, calling the getAssetPrice function in the ParaSpaceOracle contract can execute price = uint256(source.latestAnswer()), where source is a Chainlink aggregator. If the answer returned by the Chainlink aggregator is 0, a fallback oracle can then be used for setting price. However, besides that Chainlink's latestAnswer function is deprecated, only verifying if the returned answer is positive is also not enough to guarantee that the returned answer is not stale. Using a stale answer as price can have unexpected consequences, such as mistakenly considering a user position unhealthy that allows liquidation.

https://github.com/code-423n4/2022-11-paraspace/blob/main/paraspace-core/contracts/misc/ParaSpaceOracle.sol#L115-L136

function getAssetPrice(address asset)
    public
    view
    override
    returns (uint256)
{
    if (asset == BASE_CURRENCY) {
        return BASE_CURRENCY_UNIT;
    }

    uint256 price = 0;
    IEACAggregatorProxy source = IEACAggregatorProxy(assetsSources[asset]);
    if (address(source) != address(0)) {
        price = uint256(source.latestAnswer());
    }
    if (price == 0 && address(_fallbackOracle) != address(0)) {
        price = _fallbackOracle.getAssetPrice(asset);
    }

    require(price != 0, Errors.ORACLE_PRICE_NOT_READY);
    return price;
}

To avoid using a stale answer returned by the Chainlink aggregator, according to Chainlink's documentation:

The latestRoundData function can be used instead of the deprecated latestAnswer function. roundId and answeredInRound are also returned. "You can check answeredInRound against the current roundId. If answeredInRound is less than roundId, the answer is being carried over. If answeredInRound is equal to roundId, then the answer is fresh." "A read can revert if the caller is requesting the details of a round that was invalid or has not yet been answered. If you are deriving a round ID without having observed it before, the round might not be complete. To check the round, validate that the timestamp on that round is not 0." Hence, as a mitigation, https://github.com/code-423n4/2022-11-paraspace/blob/main/paraspace-core/contracts/misc/ParaSpaceOracle.sol#L127-L129 can be updated to the following code.

    if (address(source) != address(0)) {
        (uint80 roundId, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = source.latestRoundData();
        require(answeredInRound >= roundId, "answer is stale");
        require(updatedAt > 0, "round is incomplete");
        require(answer > 0, "Invalid answer");

        price = uint256(answer);            
    }
c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #5

c4-judge commented 1 year ago

dmvt marked the issue as satisfactory