code-423n4 / 2023-09-goodentry-mitigation-findings

0 stars 0 forks source link

M-05 MitigationConfirmed #38

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

Vulnerability details

Comments

The purpose of dust is to ensure that the liquidity of deposits is high enough so that the liquidity can fully repay the debt. However, there is a problem with the calculation of dust. As shown in the POC in the report, as the tick fluctuates, the actual calculated dust amount is too low or too high, and may even revert.

Mitigation

    // Round added liquidity up to expectedAmount if the difference is dust
    // ie. underlying amounts of liquidity difference is 0, or value is lower than 1 unit of token0 or token1
    if (lpAmt < expectedAmount){
      uint missingLiq = expectedAmount - lpAmt;
      uint missingLiqValue = missingLiq * latestAnswer() / 1e18;
      (uint u0, uint u1) = getTokenAmounts(missingLiq);
      uint val0 = u0 * ORACLE.getAssetPrice(address(TOKEN0.token)) / 10**TOKEN0.decimals;
      uint val1 = u1 * ORACLE.getAssetPrice(address(TOKEN1.token)) / 10**TOKEN1.decimals;
      // missing liquidity has no value, or underlying amount is 1 or less (meaning some 1 unit rounding error on low decimal tokens)
      if (missingLiqValue == 0 || (u0 <= 1 && u1 <= 1)){
        lpAmt = expectedAmount;
      }
      (u0, u1) = getTokenAmountsExcludingFees(expectedAmount);
    }

The protocol rewrites this part of the logic and solves this problem by verifying the result after adding liquidity instead of adding dust beforehand. Additionally, TokenisableRange fees are no longer compounded directly in TR, but are instead sent to the corresponding GeVault for management

Suggestion

      uint val0 = u0 * ORACLE.getAssetPrice(address(TOKEN0.token)) / 10**TOKEN0.decimals;
      uint val1 = u1 * ORACLE.getAssetPrice(address(TOKEN1.token)) / 10**TOKEN1.decimals;

      (u0, u1) = getTokenAmountsExcludingFees(expectedAmount);

These lines of code can be removed

Conclusion

LGTM

c4-judge commented 12 months ago

gzeon-c4 marked the issue as confirmed for report

gzeon-c4 commented 12 months ago

Would also consdier xuwinnie as satisfactory on this issue despite https://github.com/code-423n4/2023-09-goodentry-mitigation-findings/issues/51

c4-judge commented 12 months ago

gzeon-c4 marked the issue as satisfactory