code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L646 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L650

Vulnerability details

Impact

If a negative value is returned (< 0) from chainlink oracle and the value is cast to type uint256, the resulting value will be the unsigned representation of that value which will be an inaccurate price. Also, cases where sdprice can't be less than 0 will revert (i.e division by 0 - https://github.com/code-423n4/2023-06-stader/blob/main/contracts/SDCollateral.sol#L212)

Proof of Concept

missing check: `(, int256 totalETHBalanceInInt, , , ) = AggregatorV3Interface(staderConfig.getETHBalancePORFeedProxy()) .latestRoundData(); (, int256 totalETHXSupplyInInt, , , ) = AggregatorV3Interface(staderConfig.getETHXSupplyPORFeedProxy()) .latestRoundData(); // @audit // if ( // totalETHBalanceInInt <= 0 || totalETHBalanceInInt <= 0 // ) revert StaderOracle_BadFeed();

return (uint256(totalETHBalanceInInt), uint256(totalETHXSupplyInInt), block.number);`

Tools Used

Recommended Mitigation Steps

Always validate price feed answers before using and also it's important to note that casting a negative value of signed integer to an unsigned integer type may result in unexpected behavior, add the missing check above: if ( // totalETHBalanceInInt <= 0 || totalETHBalanceInInt <= 0 // ) revert StaderOracle_BadFeed(); and Always exercise caution when performing such type conversions to ensure that the resulting value is appropriate

Assessed type

Oracle

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #15