code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In Payment.sol contract unwrapWETH and sweepToken functions are without any access control. They are public and and doesn't validate that it's being called by any permissioned account. The result is that anyone can steal tokens.

Proof of Concept

25: 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);
        }
  }

35:  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);
        }
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Add access control to these functions.

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