code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/0cb05a462e78c4470662e9d9a4f9ab587f266bb5/contracts/utils/TradingLibrary.sol#L113-L114

Vulnerability details

Impact

Lack of validation on Chainlink price feeds may result in incorrectly functioning or non-functioning protocol.

For example:

during high volatility a price feed may be suspended or become stale;
on L2 networks the sequencer might be down
on L2 networks the sequencer might be just restarted and a grace period should be waited for results to become reliable.

The results of using a deprecated API (as reported in my other issue opened related to Chainlink API) does not give a predictable response in these cases (may be 0, may be stale, may revert?).

Proof of Concept

https://github.com/code-423n4/2022-12-tigris/blob/0cb05a462e78c4470662e9d9a4f9ab587f266bb5/contracts/utils/TradingLibrary.sol#L113-L114

            int256 assetChainlinkPriceInt = IPrice(_chainlinkFeed).latestAnswer();
            if (assetChainlinkPriceInt != 0) {
                ...
            }

Tools Used

Manual review

Recommended Mitigation Steps

Use latestRoundData() API
Use response's updatedAt (timestamp of when the round was updated) to verify the feed is not stale
Refer to Chainlink documentation regarding sequencer handling with Arbitrum/Optimism L2 network

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #655