code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/main/src/periphery/Payment.sol#L25-L46

Vulnerability details

Impact

In Payment.sol contract, there is no access control on unwrapWETH, sweepToken and refundETH. So anyone can call these method to steal the Eth.

Proof of Concept

File: src/periphery/Payment.sol

  function unwrapWETH(uint256 amountMinimum, address recipient) public payable {
    uint256 balanceWETH = Balance.balance(weth);
    if (balanceWETH < amountMinimum) revert InsufficientOutputError();

    if (balanceWETH > 0) {
      IWETH9(weth).withdraw(balanceWETH);
      SafeTransferLib.safeTransferETH(recipient, balanceWETH);
    }
  }

  function sweepToken(address token, uint256 amountMinimum, address recipient) public payable {
    uint256 balanceToken = Balance.balance(token);
    if (balanceToken < amountMinimum) revert InsufficientOutputError();

    if (balanceToken > 0) {
      SafeTransferLib.safeTransfer(token, recipient, balanceToken);
    }
  }

  function refundETH() external payable {
    if (address(this).balance > 0) SafeTransferLib.safeTransferETH(msg.sender, address(this).balance);
  }

Link to Code

You can see there is no Access Control on these functions and they are public or external. LendgineRouter and LiquidityManager inherits Payment, so these functions are externally exposed to everyone. So any balance can be stealed by anyone.

Tools Used

Manual Review

Recommended Mitigation Steps

By providing an access control the issues can be mitigated. Another Option is to remove these functions as they are not utilised by any other function.

berndartmueller commented 1 year ago

The Payment abstract utility contract is only used by the LendgineRouter and the LiquidityManager contracts, which both do not intend to hold any token funds directly.

If there's leftover WETH in those two contracts, it's not systematic and only because a previous user mistakenly transferred the incorrect amount. Additionally, the protocol is permissionless, there's no contract owner.

Closing as invalid.

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid

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