code-423n4 / 2023-01-ondo-findings

0 stars 0 forks source link

`JumpRateModelV2` may return wrong values #368

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70//contracts/lending/JumpRateModelV2.sol#L152-L162 https://github.com/code-423n4/2023-01-ondo/blob/12537cc94f4b0e9b213d53906aa2f95e425dd899/contracts/lending/CompoundLens.sol#L65

Vulnerability details

JumpRateModelV2 may return wrong values

Impact

Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision.

Vulnerability Details

In general, this is a problem due to precision mostly if affect a value where assets are involved.

In this case it is used to get supply rater per block value in JumpRateModelV2

If this value is later called in a function that uses this value for moving assets, this would cause lose of funds when solidity truncate

For example this method is called when getting cTokenMetadata values.

Tools

Manual analysis

Proof of Concept

Lack of precision due to the order of operations

  function getSupplyRate(
    uint cash,
    uint borrows,
    uint reserves,
    uint reserveFactorMantissa
  ) public view returns (uint) {
    uint oneMinusReserveFactor = uint(1e18).sub(reserveFactorMantissa);
    uint borrowRate = getBorrowRate(cash, borrows, reserves);
    uint rateToPool = borrowRate.mul(oneMinusReserveFactor).div(1e18); //@audit first divide
    return utilizationRate(cash, borrows, reserves).mul(rateToPool).div(1e18);//@audit then multiply
  }

Recommended Mitigation Steps

Reorder the operations, first division can be added to the final division, as rateToPool is only used there

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid

trust1995 commented 1 year ago

Report references line with multiplication before division which is correct.