Skip to content

Latest commit

 

History

History
70 lines (61 loc) · 2.19 KB

File metadata and controls

70 lines (61 loc) · 2.19 KB

Brisk Mango Starfish

Medium

Small loss of shares for Users due to how fee is calculated in previewWithdraw.

Summary

In UsualX, a amount of withdrawal fees is taken in withdraw and redeem. In previewWithdraw ,fee calculation is different than in withdraw function which causes fees calculated from previewWithdraw to be greater than in withdraw due to subtracting withdrawFeesBps.

uint256 fee = Math.mulDiv(
      assets,
      $.withdrawFeeBps,
@>    BASIS_POINT_BASE - $.withdrawFeeBps, 
      Math.Rounding.Ceil
    );

Root Cause

In previewWithdraw , deducting withdrawFeesBps from BASIS_POINT_BASE increase extra fee.

Internal pre-conditions

_NO_RESPONSE

External pre-conditions

_NO_RESPONSE

Attack Path

  1. User deposit 100 tokens gets 100 shares.
  2. Consider withdrawFeeBps = 500 // 5% , User withdraw 50 tokens.
  3. User receive 50 tokens ,fee = 6 tokens , left shares = 100 -( 50 + 6 ) = 44 tokens. The state for yeild.totalDeposits will be updates but it deducts 5 tokens as the added fees is 6 tokens in previewWithdraw.

impact

  • user will always pay an extra share when withdrawing.

POC

Fess calculation of previewWithdraw and withdraw are different. Consider withdrawFeeBPs = 500 , same values as in attack path. previewWithdraw

uint256 fee = Math.mulDiv(
      assets,
      $.withdrawFeeBps,
@>    BASIS_POINT_BASE - $.withdrawFeeBps, 
      Math.Rounding.Ceil
    );

    fee = (100 * 500)/ (10000 - 500 )= 50000 / 9500 = 5.2 = 6.

withdraw

    uint256 fee = Math.mulDiv(
      assets,
      $.withdrawFeeBps,
      BASIS_POINT_BASE,
      Math.Rounding.Ceil
    );

    fee = (100 * 500)/ (10000 )= 50000 / 10000 = 5.

MIGATION

remove the withdrawFeeBps deducted in BASIS_POINT_BASE.

uint256 fee = Math.mulDiv(
      assets,
      $.withdrawFeeBps,
--    BASIS_POINT_BASE - $.withdrawFeeBps,
++    BASIS_POINT_BASE , 
      Math.Rounding.Ceil
    );