code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/LiquidityManager.sol#L104-L114

Vulnerability details

Impact

Detailed description of the impact of this finding. The LiquidityManager.pairMintCallback() might be exploited to steal funding from another user due to three issues: 1) LiquidityManager.pairMintCallback() is a callback function for the addLiquidity(), which requires a user to approve the LiquidityManage some amount of allowance on token0 and token1.

2) If a malicious user can call LiquidityManager.pairMintCallback() directly, then she/he can steal fund from another liquidity providor. This can happen when the contract factory is compromised.

3) LiquidityManager.pairMintCallback() has no detection that factory is original or compromised.

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 how the LiquidityManager.pairMintCallback() might be exploited to steal funding from another user:

1) Suppose a liquidity provider Alice likes to call addLiquidity(), but before that she approves 10M of token0 and 5M of token1 to the LiquidityManager.

2) A malicious user obtains the account of the deployer of factory and then use recreat2 to deploy a malicious factory, and from the malicious factory, a new malicious lendgine contract is deployed at the same address of an old one using recreat2.

3) The malicious Lendgine can call the LiquidityManager.pairMintCallback() directly, stealing fund from Alice using the following lines:

 if (decoded.amount0 > 0) pay(decoded.token0, decoded.payer, msg.sender, decoded.amount0);
    if (decoded.amount1 > 0) pay(decoded.token1, decoded.payer, msg.sender, decoded.amount1);

where decoded.payer is Alice's address and now all 10M of token0 and 5M of token1 will be sent to the malicious Lendgine.

Tools Used

Remix

Recommended Mitigation Steps

Use EXTCODEHASH (https://soliditydeveloper.com/extcodehash) to check that the codeHash of the calculated address is indeed to the codeHash for the Lendgine by a looking up of codeHash in the factory.

kyscott18 commented 1 year ago

How would a malicious factory be deployed? Attempting to deploy the factory to the same address that already exists would revert because a contract is already deployed to that address.

c4-sponsor commented 1 year ago

kyscott18 requested judge review

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid

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