Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mint() function logic will break with fee-on-transfer(deflationary) tokens #261

Closed
code423n4 opened this issue Feb 1, 2023 · 2 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-263 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/Lendgine.sol#L99

Vulnerability details

Impact

with deflationary token mint function never succeed

Proof of Concept

mint() function checking if (balanceAfter < balanceBefore + collateral) revert InsufficientInputError();
i.e balanceAfter should greater or equal to balanceBefore + collateral

But in case of fee-on transfer tokens some amount will burn from sending amount
i.e collateral that sent > collateral that received

so balanceAfter is always less than balanceBefore + collateral in case of fee-on-transfer tokens

function mint(
    address to,
    uint256 collateral,
    bytes calldata data
  )
    external
    override
    nonReentrant
    returns (uint256 shares)
  {
    _accrueInterest();

    uint256 liquidity = convertCollateralToLiquidity(collateral);
    shares = convertLiquidityToShare(liquidity);

    if (collateral == 0 || liquidity == 0 || shares == 0) revert InputError();
    if (liquidity > totalLiquidity) revert CompleteUtilizationError();
    // next check is for the case when liquidity is borrowed but then was completely accrued
    if (totalSupply > 0 && totalLiquidityBorrowed == 0) revert CompleteUtilizationError();

    totalLiquidityBorrowed += liquidity;  // @audit 36
    (uint256 amount0, uint256 amount1) = burn(to, liquidity); // @audit same function name
    _mint(to, shares);

    uint256 balanceBefore = Balance.balance(token1);
    IMintCallback(msg.sender).mintCallback(collateral, amount0, amount1, liquidity, data);
    uint256 balanceAfter = Balance.balance(token1);

    if (balanceAfter < balanceBefore + collateral) revert InsufficientInputError();   // @audit-issue this logic will breake with defalmantionary tokens

    emit Mint(msg.sender, collateral, shares, liquidity, to);
  }

Tools Used

Manual review

Recommended Mitigation Steps

Some logic change should made to support fee-on-transfer tokens

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Feb 1, 2023
code423n4 added a commit that referenced this issue Feb 1, 2023
@c4-judge c4-judge closed this as completed Feb 6, 2023
@c4-judge
Copy link

c4-judge commented Feb 6, 2023

berndartmueller marked the issue as duplicate of #263

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Feb 16, 2023
@c4-judge
Copy link

berndartmueller marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-263 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants