code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

There are 3 areas of interests in this function where rounding errors and inconsistencies may occur. Rounding during Curve Value Calculation, Rounding during Factor Increase Calculation and Rounding during Protocol Fee Calculation.

Proof of Concept

Rounding during Curve Value Calculation

Rounding errors can occur when converting between different numerical representations, such as converting from int256 to uint192. These errors can accumulate over time and affect the accuracy of interest calculations, leading to unintended consequences in the contract's behavior.

int256 _reserveRatio = int256(_ui18);
int256 _intCurveVal = curveMaster.getValueAt(address(0x00), _reserveRatio);

Rounding during Factor Increase Calculation:

uint192 _e18FactorIncrease = _safeu192(
    _truncate(
        _truncate(
            (uint256(_timeDifference) *
                uint256(1e18) *
                uint256(_curveVal)) / (365 days + 6 hours)
        ) * uint256(interest.factor)
    )
);

In this part, multiple arithmetic operations are performed, which may introduce rounding errors.

Rounding during Protocol Fee Calculation:

uint192 _protocolAmount = _safeu192(
    _truncate(
        uint256(_valueAfter - _valueBefore) * uint256(protocolFee)
    )
);

Tools Used

Manual Review

Recommended Mitigation Steps

Use SafeMath: Replace all direct arithmetic operations with SafeMath functions. SafeMath provides protection against overflows and underflows and ensures that calculations are performed with higher precision.

Higher Precision: Use higher precision data types, where necessary, to store intermediate results. For example, instead of uint192, consider using uint256 to store intermediate values.

Consistent Units: Be consistent with units throughout the calculations. Avoid mixing different units of measurements, as it can lead to confusion and errors.

Avoid Type Conversions: Minimize type conversions between different numerical representations, as each conversion can introduce rounding errors.

Assessed type

Math

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #138

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)