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

0 stars 0 forks source link

Malicious user can create a dummy `Lendgine` contract by mimicing a salt with same encoding format but using a malicious AMM invariant function #279

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/LiquidityManager.sol#L136 https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Factory.sol#L82

Vulnerability details

Impact

Lendgine contract address is created using a salt that is generated by a hash of pool parameters in Factory.sol. A malicious user can create a Lendgine exploit contract that uses the salt generated by exact same encoding but this contract inherits a Pair contract with a malicious custom invariant function.

Key vulnerability is that when LP deposits funds into the pool using the addLiquidity function in LiquidityManager, code does not cross check if this address is in the getLendgine mapping in Factory.sol. If a malicious contract is compliant with ILendgine interface, it will go undetected in the LiquidityManager functions.

When a user tries to add liquidity using the addLiquidity function in LiquidityManager wrapper, computed address for lendgine contract points to the exploited Lendgine contract. Note that this contract exactly can mimic the original contract with same function signatures & storage states but using a custom AMM invariant function.

Proof of Concept

Tools Used

Manual

Recommended Mitigation Steps

Before using Lendgine address generated by computeAddress function, verify if this address exists in the getLendgine mapping. Revert otherwise.

berndartmueller commented 1 year ago

The create2 opcode (used when using Solidity's salt keyword) takes the address of the creating contract (Factory) and the given salt to determine the address for the new contract. Thus, Bob can not deploy a malicious Lendgine contract to an arbitrary address which would be used by the LiquidityManager. For a more detailed explanation, see https://ethereum.stackexchange.com/a/134143

Closing as invalid.

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid