code-423n4 / 2024-04-renzo-validation

2 stars 2 forks source link

Unsafe typecasting may result in various disruptions to protocol operations #212

Closed c4-bot-6 closed 4 months ago

c4-bot-6 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L2/Oracle/RenzoOracleL2.sol#L54

Vulnerability details

Impact

The unsafe typecast vulnerability in the getMintRate() function could lead to very inaccurate token prices, potentially causing financial losses for users and disruptions to the protocol's operations. Additionally, if the price returned from Chainlink has negative value this may render the functionality dependent from this unusable, as the direct typecast to unsigned integer will return really huge number - potentially leading to Denial-of-Service scenarios.

Proof of Concept

The unsafe typecast vulnerability can be observed in the RenzoOracleL2::getMintRate() function.

function getMintRate() public view returns (uint256, uint256) {
    (, int256 price,, uint256 timestamp,) = oracle.latestRoundData();
    if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired();
    // scale the price to have 18 decimals
@>   uint256 _scaledPrice = (uint256(price)) * 10 ** (18 - oracle.decimals()); // @audit-issue unsafe typecast
    if (_scaledPrice < 1 ether) revert InvalidOraclePrice();
    return (_scaledPrice, timestamp);
}

This vulnerability could occur if the Chainlink price feed returns a negative value for the price variable. While the likelihood of this happening is quite rare, it is still possible due to the returned type being int256 and not uint256. If this situation does occur, it will result in very unexpected and inaccurate token prices. For example, if we cast -1000 to uint256, we receive 115792089237316195423570985008687907853269984665640564039457584007913129638936.

The chance of this issue occurring may be rare, but smart contracts should be developed to prioritize safety as much as possible and not assume that rare occurrences will never happen.

Tools Used

VSCode

Recommended Mitigation Steps

Add a check to ensure that the returned price from the Chainlink oracle is not negative. This check can be implemented similar to the one used in the RenzoOracle::lookupTokenValue() function:

(, int256 price, , uint256 timestamp, ) = oracle.latestRoundData();
 if (price <= 0) revert InvalidOraclePrice();

Assessed type

Invalid Validation

DadeKuma commented 4 months ago

@howlbot reject