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

3 stars 2 forks source link

the '_calculate' function in AMPHClaimer.sol may have rounding error vulnerability due to the use of floating-point arithmetic with 1e6 precision #399

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/core/AMPHClaimer.sol#L222

Vulnerability details

Impact

The ‘_calculate’ function in the AMPHClaimer.sol contract may have rounding error vulnerabilities due to the use of floating-point arithmetic with 1e6 precision.

Rounding errors occur when performing arithmetic operations involving floating-point numbers. In the ‘_calculate’ function, some calculations involve multiplying and then dividing values by 1e6, which introduces rounding errors.

In this code: _amphForThisTurn = ((_rate * _tempAmountReceived) / 1e12) / 1e6; // 1e6

Proof of Concept

In the code above, the ‘_rate’ is represented with 1e6 precision (6 decimal places), and ‘_tempAmountReceived’ has 1e18 precision (18 decimal places). The division by 1e12 is to prevent an intermediate overflow, but it introduces rounding errors. Similarly, the final division by 1e6 converts the result to 1e18 precision, again introducing rounding errors and also unintended results due to overflow or underflow.

The rounding errors could lead to inaccuracies in the calculated amphForThisTurn, amphToMint, and potentially the overall amount of AMPH tokens minted.

Contract name: AMPHClaimer.sol

Code link: https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/core/AMPHClaimer.sol#222

Code line: _amphForThisTurn = ((_rate * _tempAmountReceived) / 1e12) / 1e6; // 1e6

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate rounding errors, consider using integer arithmetic instead of floating-point arithmetic. You could convert the calculations to integer operations by multiplying the values with the appropriate factors to move the decimal points to an integer precision (e.g., 1e18). This approach would reduce the chances of encountering rounding errors. This way:

function calculateAmount(uint256 inputValue) internal pure returns (uint256) { // Perform fixed-point arithmetic using the precision constant uint256 result = multiply(inputValue, 123456789); // Multiply by a constant result = divide(result, 987654321); // Divide by another constant return result; }

Also, using fixed-point arithmetic with integer values, you can avoid rounding errors and achieve precise calculations in your smart contracts. This way:

uint256 internal constant PRECISION = 1e18;

amphForThisTurn = (tempAmountReceived * rate) / PRECISION; The ‘PRECISION’ constant with a value of 1e18 is introduced, representing 18 decimal places. All calculations involving rates and amounts have been adapted to use fixed-point arithmetic by multiplying and dividing by ‘PRECISION.’

Assessed type

Decimal

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Insufficient quality