cheatcode - Rounding Errors in Entrance Fee Calculation lead to Precision Loss

[sherlock-admin]

cheatcode

medium

Rounding Errors in Entrance Fee Calculation lead to Precision Loss

Summary

In LiquidityBorrowingManager contract rate calculations are done using the FullMath library in some places, which properly handles precision. However, there are still unmitigated rounding risks in parts that don't use FullMath.

Vulnerability Detail

The daily rate collateral, for example, is calculated using FullMath in _precalculateBorrowing()

cache.dailyRateCollateral = FullMath.mulDivRoundingUp(
    cache.borrowedAmount, 
    cache.dailyRateCollateral,
    Constants.BP
);

This properly handles precision by rounding up. However, later in the same function, the hold token entrance fee is calculated without FullMath:

cache.holdTokenEntraceFee = 
    (cache.holdTokenBalance * cache.holdTokenEntraceFee * Constants.COLLATERAL_BALANCE_PRECISION) / Constants.BP;

While COLLATERAL_BALANCE_PRECISION allows for high precision in intermediate calculations, any operation that requires scaling the value back down to its original magnitude such as when finalizing a fee amount can introduce truncation errors.

Impact

The rounding risk could lead to small losses compounding over time.

Steps to Reproduce

1. A user starts the borrowing process, triggering the _precalculateBorrowing() function.

2. The contract calculates the entrance fee based on the current implementation, which could lead to precision loss. For example, if the actual balance involved fractional tokens or if the fee calculation was meant to be more granular, the current method might not capture the fractional part due to Solidity's rounding down behavior.

3. To see the flaw, compare the result with a method that uses FullMath.mulDivRoundingUp for the calculation, which could handle fractional parts more accurately:

cache.holdTokenEntraceFee = FullMath.mulDivRoundingUp(cache.holdTokenBalance, cache.holdTokenEntraceFee, Constants.BP);

In transactions where the entrance fee calculation involves fractional tokens, the discrepancy becomes apparent. The original method might round down and miss collecting sufficient fees, while the corrected method ensures that the fee is rounded up, capturing the intended fee amount even when dealing with fractional amounts.

Code Snippet

https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L970C9-L975C1

Tool used

Manual Review

Recommendation

Use the FullMath library for consistency and precision

cache.holdTokenEntraceFee = FullMath.mulDivRoundingUp(cache.holdTokenBalance, cache.holdTokenEntraceFee, Constants.BP);

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

POC?

[nevillehuang]

Invalid, lack required PoC as noted in sherlock rules to demonstrate impact of finding. Additionally, as noted by the watson computation is performed in COLLATERAL_BALANCE_PRECISION (18 decimals), so I believe the possible rounding errors are extremely minute.