code-423n4 / 2023-07-amphora-findings

3 stars 2 forks source link

Possibility of Precision Error #361

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/main/core/solidity/contracts/periphery/oracles/UniswapV3OracleRelay.sol#L85

Vulnerability details

Impact

Precision error of _e18Amount return in _toBase18(...) function of L84 of UniswapV3OracleRelay.sol contract is possible when _decimal is greater than 18 due to missing validation check of _amount and _decimals parameters. This could have bad effect on the Oracle efficient Contract execution.

Proof of Concept

https://github.com/code-423n4/2023-07-amphora/blob/main/core/solidity/contracts/periphery/oracles/UniswapV3OracleRelay.sol#L84-L86

84. function _toBase18(uint256 _amount, uint8 _decimals) internal pure returns (uint256 _e18Amount) {
85.  _e18Amount = (_decimals > 18) ? _amount / (10 ** (_decimals - 18)) : _amount * (10 ** (18 - _decimals));
86. }

from L85 when _decimals is greater than 18 the next calculation (_amount / (10 (_decimals - 18))) needs to check if the digits of "_amount" is efficiently more than the digits from the result of (10 (_decimals - 18)) before the division takes place to avoid unnecessary decimal numbers which is not allowed in solidity and could result to error. e.g if _amount is 100 and _decimals is 24 then the calculation would be (100 / (10 ** (24 - 18))) = (100 / (1000000)) = 0.0001 but this result would be written as zero due to precision error

Tools Used

Solidity, Manual Review

Recommended Mitigation Steps

A Validation check to confirm _amount digits is greater than (_decimals - 18) or a safeMath library would solve this e.g

84. function _toBase18(uint256 _amount, uint8 _decimals) internal pure returns (uint256 _e18Amount) {
85.  _e18Amount = (_decimals > 18) ? 
        ( _amount.decimalFunc() > (_decimals - 18)? _amount / (10 ** (_decimals - 18)):revert())  :
         _amount * (10 ** (18 - _decimals));
86. }

Assessed type

Error

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

minhquanym commented 1 year ago

Seems invalid

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid