code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/LendgineRouter.sol#L11 https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/LendgineRouter.sol#L228 https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Pair.sol#L102 https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Pair.sol#L103 https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Pair.sol#L122 https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Pair.sol#L123

Vulnerability details

Losses in `Pair` and `LendgineRouter` can be generated if used with ERC20 Tokens with fee on transfer

Summary

Some tokens (token1, token0, ...) are used over the code that can be any kind of ERC20 token. If this token includes fees on transfer, some operations will make the protocol lose value using wrong amounts.

Vulnerability Detail

There are ERC20 tokens that charge fee for every transfer() / transferFrom(). Some reference of this common issue: 1, 2, 3.

LendgineRouter#mintCallback assumes that the received amount is the same as the transfer amount, and uses it to calculate assets like collateralIn, etc. While the actual transferred amount can be lower for those tokens.

For example:

    SafeTransferLib.safeTransfer(decoded.token1, msg.sender, amount1);

    // pull the rest of tokens from the user
    uint256 collateralIn = collateralTotal - amount1 - collateralSwap;//@audit amount1 can be more than it actual is
    if (collateralIn > decoded.collateralMax) revert AmountError();

    pay(decoded.token1, decoded.payer, msg.sender, collateralIn);

Impact

Losses of assets because of wrong amounts used in tokens with fees

Code Snippet

LendgineRouter.sol#L117 SafeTransferLib.safeTransfer(decoded.token1, msg.sender, amount1);
LendgineRouter.sol#L228 SafeTransferLib.safeTransfer(decoded.token1, msg.sender, amount1);
Pair.sol#L102 if (amount0 > 0) SafeTransferLib.safeTransfer(token0, to, amount0);
Pair.sol#L103 if (amount1 > 0) SafeTransferLib.safeTransfer(token1, to, amount1);
Pair.sol#L122 if (amount0Out > 0) SafeTransferLib.safeTransfer(token0, to, amount0Out);
Pair.sol#L123 if (amount1Out > 0) SafeTransferLib.safeTransfer(token1, to, amount1Out);

Tool used

Manual Review

Recommendation

Consider comparing before and after balance to get the actual transferred amount.

c4-judge commented 1 year ago

berndartmueller marked the issue as duplicate of #263