Bitter Rouge Alpaca
High
The OracleUtils tokenToEth()
and ethToToken()
perform division before multiplication, causing uncessary loss of precision.
For explanation, we'll consider tokenToEth()
function,
https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/libraries/OracleUtils.sol#L157
function tokenToEth(
uint256 _amount,
address _pricefeed,
uint128 _chainlink_heartbeat,
uint256 _decimals
) public view checkSequencerActive returns (uint256 EthValue) {
...snip...
//if ETH is on the left side of the fraction in the price feed
if (ethLeftSide(_pricefeed)) {
EthValue = FullMath.mulDiv(
_amount,
10 ** AggregatorV3Interface(_pricefeed).decimals(),
uint256(price)
);
} else {
EthValue = FullMath.mulDiv(
_amount,
uint256(price),
10 ** AggregatorV3Interface(_pricefeed).decimals() // @audit-issue division before multiplication
);
}
// audit fix
EthValue = EthValue * 10 ** (18 - _decimals); // @audit-issue multiplication after division
}
As can be seen, the calculated EthValue
in if/else block above, has been multiplied with 10 ** (18 - _decimals)
, which is done right after the division. The division before multiplication cause unecessary roundings, result in value loss.
No response
No response
No response
Precision loss in ethToToken/tokenToEth()
.
Create a test file, under contracts/Test/utils/OracleUtilsTest.t.sol
and run forge test --via-ir --mt testPrecisionLoss --fork-block-number 21413043 --fork-url https://eth-mainnet.g.alchemy.com/v2/cmnVRZ7q4nn5moCWCvUbnRANwtPM1VOs -vv
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.20;
import "forge-std/Test.sol";
import "../../libraries/OracleUtils.sol";
contract OracleUtilsTest is Test , OracleUtils {
OracleUtils public util;
address public constant BTC_ETH = 0xdeb288F737066589598e9214E782fa5A8eD689e8;
address public constant ETH_BTC = 0xAc559F25B1619171CbC396a50854A3240b6A4e99;
constructor() OracleUtils(address(0)) {}
function setUp() public {
util = new OracleUtils(address(0));
}
function testPrecisionLoss() public {
uint256 btcAmount = 100e8;
uint256 btcDecimal = 8;
uint256 ethDecimal = 18;
uint256 ethAmountWithPrecisionLoss = tokenToEth(btcAmount, BTC_ETH, uint128(block.timestamp), btcDecimal);
uint256 ethAmountWithoutPrecisionLoss = tokenToEthCorrected(btcAmount, BTC_ETH, uint128(block.timestamp), btcDecimal);
console.log("ETH Amount with precision loss: ", ethAmountWithPrecisionLoss);
console.log("ETH Amount without precision loss: ", ethAmountWithoutPrecisionLoss);
console.log("diff: ", ethAmountWithoutPrecisionLoss - ethAmountWithPrecisionLoss);
}
function tokenToEthCorrected(
uint256 _amount,
address _pricefeed,
uint128 _chainlink_heartbeat,
uint256 _decimals
) public view checkSequencerActive returns (uint256 EthValue) {
(
uint80 roundID,
int256 price,
,
uint256 timeStamp,
uint80 answeredInRound
) = AggregatorV3Interface(_pricefeed).latestRoundData();
// heartbeat check
require(
timeStamp >= block.timestamp - _chainlink_heartbeat,
"Stale pricefeed"
);
// minAnswer/maxAnswer check
IChainlinkAggregator aggregator = IChainlinkAggregator(
IChainlinkPriceFeed(_pricefeed).aggregator()
);
require(
((price > int256(aggregator.minAnswer())) &&
(price < int256(aggregator.maxAnswer()))),
"min/max reached"
);
require(answeredInRound >= roundID, "Answer given before round");
//if ETH is on the left side of the fraction in the price feed
if (ethLeftSide(_pricefeed)) {
EthValue = FullMath.mulDiv(
_amount * 10 ** AggregatorV3Interface(_pricefeed).decimals(),
10 ** (18 - _decimals),
uint256(price)
);
} else {
EthValue = FullMath.mulDiv(
_amount * uint256(price),
10 ** (18 - _decimals),
10 ** AggregatorV3Interface(_pricefeed).decimals()
);
}
}
}
Perform multiplication before division, corrected version for tokenToEth()
is shown in PoC above,