code-423n4 / 2023-10-opendollar-findings

10 stars 7 forks source link

Integer Overflow Risk in Financial Calculation #389

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/proxies/actions/BasicActions.sol#L58-L65

Vulnerability details

Impact

When specific input values are used, these functions can produce incorrect results due to exceeding the representation range of the int256 data type. This can lead to inaccurate debt calculations, which might cause financial discrepancies or issues in the application.

Proof of Concept

uint256 _coinAmount = 5000;
uint256 _rate = 2000;

int256 _deltaDebt = (_coinAmount / _rate).toInt();

_coinAmount / _rate equals 2.5. When we cast 2.5 to int256, it becomes 2. This is a loss of precision.

This example has equivalent variations,It can happen at any time in this financial system and if this problem is not fixed, losses will result.

Tools Used

VSCode

Recommended Mitigation Steps

import "@openzeppelin/contracts/utils/math/SafeMath.sol";
// ...

using SafeMath for uint256;
using SafeMath for int256;

// ...

  function _getRepaidDeltaDebt(
    address _safeEngine,
    bytes32 _cType,
    address _safeHandler
  ) internal view returns (int256 _deltaDebt) {
    uint256 _rate = ISAFEEngine(_safeEngine).cData(_cType).accumulatedRate;
    uint256 _generatedDebt = ISAFEEngine(_safeEngine).safes(_cType, _safeHandler).generatedDebt;
    uint256 _coinAmount = ISAFEEngine(_safeEngine).coinBalance(_safeHandler);

    _deltaDebt = _coinAmount.div(_rate).toInt();
    _deltaDebt = (uint256(_deltaDebt) <= _generatedDebt) ? _deltaDebt.neg() : _generatedDebt.neg().toInt();
  }

Assessed type

Math

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #209

c4-judge commented 1 year ago

MiloTruck marked the issue as unsatisfactory: Invalid