Numoen contest - rvierdiiev's results

Lines of code

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

Vulnerability details

Impact

Rebasing tokens are not supported. Pool can be insolvent.

Proof of Concept

When depositor accrues interests, his tokensOwed amount is updated inside Position.update function. https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/libraries/Position.sol#L38-L65

  function update(
    mapping(address => Position.Info) storage self,
    address owner,
    int256 sizeDelta,
    uint256 rewardPerPosition
  )
    internal
  {
    Position.Info storage positionInfo = self[owner];
    Position.Info memory _positionInfo = positionInfo;


    uint256 tokensOwed;
    if (_positionInfo.size > 0) {
      tokensOwed = newTokensOwed(_positionInfo, rewardPerPosition);
    }


    uint256 sizeNext;
    if (sizeDelta == 0) {
      if (_positionInfo.size == 0) revert NoPositionError();
      sizeNext = _positionInfo.size;
    } else {
      sizeNext = PositionMath.addDelta(_positionInfo.size, sizeDelta);
    }


    if (sizeDelta != 0) positionInfo.size = sizeNext;
    positionInfo.rewardPerPositionPaid = rewardPerPosition;
    if (tokensOwed > 0) positionInfo.tokensOwed = _positionInfo.tokensOwed + tokensOwed;
  }

As you can see interests are accrued as newTokensOwed and they are saved as collateral token value that should be paid for user. Later he can call Lendgine.collect in order to receive interests. https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/Lendgine.sol#L194-L206

  function collect(address to, uint256 collateralRequested) external override nonReentrant returns (uint256 collateral) {
    Position.Info storage position = positions[msg.sender]; // SLOAD
    uint256 tokensOwed = position.tokensOwed;


    collateral = collateralRequested > tokensOwed ? tokensOwed : collateralRequested;


    if (collateral > 0) {
      position.tokensOwed = tokensOwed - collateral; // SSTORE
      SafeTransferLib.safeTransfer(token1, to, collateral);
    }


    emit Collect(msg.sender, to, collateral);
  }

In case when rebasing token will be used as collateral such approach will not work as at the moment when tokensOwed was increased by value X of rebasing token, this value can become Y < X, when user calls collect. As result he will receive more tokens from the pool that he should. In case if token increased in price, then user will receive less tokens as interests.

Tools Used

VsCode

Recommended Mitigation Steps

I believe that factory should have list if whitelisted tokens that can be used by protocol. Currently any token can be used.

Lines of code

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

Vulnerability details

Impact

Lendgine.mint doesn't return overpaid collateral and doesn't provide more liquidity for him.

Proof of Concept

When borrower wants to get liquidity from the pool he calls Lendgine.mint function. https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/Lendgine.sol#L71-L102

  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;
    (uint256 amount0, uint256 amount1) = burn(to, liquidity);
    _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();


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

This function calculates amount of liquidity that will be borrowed to the caller, mints him propotional amount of shares and then calls IMintCallback in order to receive collateral from user. if (balanceAfter < balanceBefore + collateral) revert InsufficientInputError(); This is how amount received from user is checked. It's possible that user will pay more collateral actually. And in this case nothing is sent back to him and also he doesn't receive more liquidity and shares.

Tools Used

VsCode

Recommended Mitigation Steps

Do not allow user to pay more then collateral amount.