Funds that are stored in LiquidityManager are at risk

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/main/src/periphery/LiquidityManager.sol#L233

Vulnerability details

Impact

Token1 stored in LiquidityManager might be stolen at any time (based on the assumption that the Balance.balance() function call will return the balance of the LiquidityManager in: https://github.com/code-423n4/2023-01-numoen/blob/main/src/periphery/Payment.sol#L36)

Proof of Concept

The recipient of the collect function call can be the address of the LiquidityManager in case of the param.recipient == address(0) : https://github.com/code-423n4/2023-01-numoen/blob/main/src/periphery/LiquidityManager.sol#L233 https://github.com/code-423n4/2023-01-numoen/blob/main/src/periphery/LiquidityManager.sol#L246 Tis means that it is possible for token1 to be transferred to LiquidityManager: https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/Lendgine.sol#L202

But because LiquidityManager inherits from Payment the tokens that were collected to the LiquidityManager can be stolen via and Payment.sweepToken() : https://github.com/code-423n4/2023-01-numoen/blob/main/src/periphery/Payment.sol#L35

Tools Used

Manual Analysis

Recommended Mitigation Steps

If possible make the functions from Payment internal so that only contracts that inherit from it can call those functions

[berndartmueller]

Leaving open for sponsor review.

If a user provides address(0) as the recipient, it is considered a user error and thus QA (Low).

[c4-judge]

berndartmueller marked the issue as duplicate of #66

[c4-judge]

berndartmueller changed the severity to QA (Quality Assurance)

[c4-judge]

berndartmueller marked the issue as grade-c