Lines of code

Vulnerability details

Impact

In the function price, there is no check that the return price that chainlink sends is >0.

  (uint80 roundId, int256 p, , uint256 updateTime, uint80 answeredInRound) = chainlinkFeed
        .latestRoundData();

    if (updateTime == 0 || answeredInRound < roundId) {
        revert StalePrice();
    }
    // Downcast is safe: uint256(-) reverts on underflow; block.timestamp assumed < 2^48
    uint48 secondsSince = uint48(block.timestamp - updateTime);
    if (secondsSince > timeout) revert StalePrice();

Even if it is not likely, Chainlink could return a wrong price will concur in loss of customers or protocol funds due to not checking the validity of the price.

Proof of Concept

1 Chainlink has some type of error while sending data
2 You receive a < 0 price or 0 in your contract
3 Depending on what the price function is being used for, more or less funds will be at risk

Tools Used

manual

Recommended Mitigation Steps

add:

       if (p < 0)
        revert Errors.NegativePrice(token, address(feed[token]));

Also as good practices:

Checking the returned answer is not 0.
Verify result is within an allowed margin of freshness by checking updatedAt.
Verify answer is indeed for the last known round.

The text was updated successfully, but these errors were encountered:

All reactions