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

0 stars 0 forks source link

The LendgineRouter.burn() will always REVERT due to the callback function forgot to send the due token0 back. #240

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/LendgineRouter.sol#L228-L237

Vulnerability details

Impact

Detailed description of the impact of this finding. The LendgineRouter.burn() will always REVERT due to the callback function forgot to send the due token0 back. The callback function pairMintCallback() is supposed to send back amount0 amount of token0 back to Lendgine, but it forgot to do so, leading to an invariant check failure, and finally, the revert of LendgineRouter.burn().

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

We show why LendgineRouter.burn() will always REVERT: 1) Suppose Alice calls LendgineRouter.burn() to take an option position and withdraw it fully into token1.

2) LendgineRouter.burn() will call Lendgine.burn() as follows:

amount = ILendgine(lendgine).burn(
      address(this),
      abi.encode(
        PairMintCallbackData({
          token0: params.token0,
          token1: params.token1,
          token0Exp: params.token0Exp,
          token1Exp: params.token1Exp,
          upperBound: params.upperBound,
          collateralMin: params.collateralMin,
          amount0Min: params.amount0Min,
          amount1Min: params.amount1Min,
          swapType: params.swapType,
          swapExtraData: params.swapExtraData,
          recipient: recipient
        })

3) Lendgine.burn() will call Lendgine.mint(liquidity, data); https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Lendgine.sol#L117

4) Lendgine.mint(liquidity, data) calls back the LendgineRouter.pairMintCallback(), expecting it to send back amount0 of token0 and amount1 of token1.

function mint(uint256 liquidity, bytes calldata data) internal {
    uint120 _reserve0 = reserve0; // SLOAD
    uint120 _reserve1 = reserve1; // SLOAD
    uint256 _totalLiquidity = totalLiquidity; // SLOAD

    uint256 balance0Before = Balance.balance(token0);
    uint256 balance1Before = Balance.balance(token1);
    IPairMintCallback(msg.sender).pairMintCallback(liquidity, data);
    uint256 amount0In = Balance.balance(token0) - balance0Before;
    uint256 amount1In = Balance.balance(token1) - balance1Before;

    if (!invariant(_reserve0 + amount0In, _reserve1 + amount1In, _totalLiquidity + liquidity)) {
      revert InvariantError();
    }

4) However, LendgineRouter.pairMintCallback() only sends back token1, and forgets to send any token0 back, even after swapping for required token0.

function pairMintCallback(uint256 liquidity, bytes calldata data) external override {
    PairMintCallbackData memory decoded = abi.decode(data, (PairMintCallbackData));

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

    uint256 r0 = ILendgine(msg.sender).reserve0();
    uint256 r1 = ILendgine(msg.sender).reserve1();
    uint256 totalLiquidity = ILendgine(msg.sender).totalLiquidity();

    uint256 amount0;
    uint256 amount1;

    if (totalLiquidity == 0) {
      amount0 = decoded.amount0Min;
      amount1 = decoded.amount1Min;
    } else {
      amount0 = FullMath.mulDivRoundingUp(liquidity, r0, totalLiquidity);
      amount1 = FullMath.mulDivRoundingUp(liquidity, r1, totalLiquidity);
    }

    if (amount0 < decoded.amount0Min || amount1 < decoded.amount1Min) revert AmountError();

    // swap for required token0
    uint256 collateralSwapped = swap(
      decoded.swapType,
      SwapParams({
        tokenIn: decoded.token1,
        tokenOut: decoded.token0,
        amount: -SafeCast.toInt256(amount0),
        recipient: msg.sender
      }),
      decoded.swapExtraData
    );

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

    // determine remaining and send to recipient
    uint256 collateralTotal = ILendgine(msg.sender).convertLiquidityToCollateral(liquidity);
    uint256 collateralOut = collateralTotal - amount1 - collateralSwapped;
    if (collateralOut < decoded.collateralMin) revert AmountError();

    if (decoded.recipient != address(this)) {
      SafeTransferLib.safeTransfer(decoded.token1, decoded.recipient, collateralOut);
    }
  }

5) As a result, Lendgine.mint(liquidity, data) will fail the invariant check and revert. So will LendgineRouter.burn() revert as well.

Tools Used

Recommended Mitigation Steps

We need to send back the due amount0 of token0 to its caller.

function pairMintCallback(uint256 liquidity, bytes calldata data) external override {
    PairMintCallbackData memory decoded = abi.decode(data, (PairMintCallbackData));

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

    uint256 r0 = ILendgine(msg.sender).reserve0();
    uint256 r1 = ILendgine(msg.sender).reserve1();
    uint256 totalLiquidity = ILendgine(msg.sender).totalLiquidity();

    uint256 amount0;
    uint256 amount1;

    if (totalLiquidity == 0) {
      amount0 = decoded.amount0Min;
      amount1 = decoded.amount1Min;
    } else {
      amount0 = FullMath.mulDivRoundingUp(liquidity, r0, totalLiquidity);
      amount1 = FullMath.mulDivRoundingUp(liquidity, r1, totalLiquidity);
    }

    if (amount0 < decoded.amount0Min || amount1 < decoded.amount1Min) revert AmountError();

    // swap for required token0
    uint256 collateralSwapped = swap(
      decoded.swapType,
      SwapParams({
        tokenIn: decoded.token1,
        tokenOut: decoded.token0,
        amount: -SafeCast.toInt256(amount0),
        recipient: msg.sender
      }),
      decoded.swapExtraData
    );

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

+ // pay token0
+ SafeTransferLib.safeTransfer(decoded.token0, msg.sender, amount0);

    // determine remaining and send to recipient
    uint256 collateralTotal = ILendgine(msg.sender).convertLiquidityToCollateral(liquidity);
    uint256 collateralOut = collateralTotal - amount1 - collateralSwapped;
    if (collateralOut < decoded.collateralMin) revert AmountError();

    if (decoded.recipient != address(this)) {
      SafeTransferLib.safeTransfer(decoded.token1, decoded.recipient, collateralOut);
    }
  }
berndartmueller commented 1 year ago

The required amount of token0 is provided to the caller msg.sender in LendgineRouter.sol#L222 as the recipient of the token swap.

Closing as invalid.

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid