code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Lendgine.sol#L91-L93 https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/LendgineRouter.sol#L86-L117

Vulnerability details

Impact

Borrower of liquidity can just keep borrowing liquidity as he will also get token0 and token1, draining the protocol funds.

Proof of Concept

Lendgine#mint calls Pair#burn before calling _mint() to mint the number of ERC20 power tokens.

totalLiquidityBorrowed += liquidity;
(uint256 amount0, uint256 amount1) = burn(to, liquidity);
_mint(to, shares);

In Pair#burn, the to parameter receives token0 and token1

if (amount0 == 0 && amount1 == 0) revert InsufficientOutputError();

if (amount0 > 0) SafeTransferLib.safeTransfer(token0, to, amount0);
if (amount1 > 0) SafeTransferLib.safeTransfer(token1, to, amount1);

Pair#burn works with Lendgine#withdraw, because a liquidity provider will 'burn' their liquidity token and get back two tokens. However, Pair#burn does not work with Lendgine#add, because the borrower who wants to mint a Power token with token1 as collateral will not only get the Power token, but also token0 and token1. The function Lendgine#mint then calls LendgineRouter#mintCallback, which transfer the necessary amount of token1 to mint an option position.

  /// @notice Transfer the necessary amount of token1 to mint an option position
  function mintCallback(
    uint256 collateralTotal,
    uint256 amount0,
    uint256 amount1,
    uint256,
    bytes calldata data
  )
    external
    override
  {
    MintCallbackData memory decoded = abi.decode(data, (MintCallbackData));

    address lendgine = LendgineAddress.computeAddress(
      factory, decoded.token0, decoded.token1, decoded.token0Exp, decoded.token1Exp, decoded.upperBound
    );
    if (lendgine != msg.sender) revert ValidationError();

    // swap all token0 to token1
    uint256 collateralSwap = swap(
      decoded.swapType,
      SwapParams({
        tokenIn: decoded.token0,
        tokenOut: decoded.token1,
        amount: SafeCast.toInt256(amount0),
        recipient: msg.sender
      }),
      decoded.swapExtraData
    );

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

When a borrower borrows liquidity, he pays two things. Firstly, he pays token1 as collateral. Secondly, he pays the equivalent borrow rate. Borrower should not get any token0 back.

Tools Used

VSCode

Recommended Mitigation Steps

When borrower is borrowing liquidity, transfer the liquidity to another contract for safekeeping first instead of transferring to the borrower. This way, the borrower will not benefit.

c4-sponsor commented 1 year ago

kyscott18 marked the issue as sponsor disputed

kyscott18 commented 1 year ago

I don't see how this is an issue. The borrower provides a certain amount of token1 as collateral and then receives the calculated amount of lp tokens because the lp tokens are borrowed against this collateral. For convenience, the lp token is withdrawn into the underlying token0 and token1 which is why these tokens are returned to the caller

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-01-numoen-findings