Closed c4-bot-2 closed 6 months ago
raymondfam marked the issue as insufficient quality report
raymondfam marked the issue as duplicate of #22
Similar to #22.
hansfriese changed the severity to QA (Quality Assurance)
hansfriese marked the issue as grade-c
Lines of code
https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L30 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L27 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L76
Vulnerability details
Impact
It is a valid use case to have price as a negative number. Solidity uses 2’s compliment hence when casting a negative int256 to a uint256 the result will be greater than 2^255, since the signed bit of a negative int256 will always be set to one.
Proof of Concept
Although the damage caused by this is very limited since if the basePrice is 0, then in
baseOracleCircuitBreaker
theinvalidFetchData
will become true and the protocol returns thetwapprice
, but if somehow thetwapprice
is also 0 , then it also reverts(although i haven't seen any chances of that till reporting this).But if somehow due to twap manipulation due to flashloan or small liquidity pool in uniswapv3, the twap also returns bad price ,then no further checks are there, which ultimately takes the price of the twap as the intended purpose.Tools Used
Manual Review
Recommended Mitigation Steps
Carry out all the calculation in int256, or change the function parameter to uint256 or change the logic of this line
uint256 basePriceInEth = basePrice > 0 ? uint256(basePrice * C.BASE_ORACLE_DECIMALS).inv() : 0;
And the best way will be to use safeCasting of OZ.Assessed type
Invalid Validation