code-423n4 / 2022-12-caviar-findings

2 stars 1 forks source link

User didn't get an lpToken when trying to add liquidity to the pair with some baseTokenAmount #465

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L63-L99

Vulnerability details

Impact

User didn't get an lpToken when trying to add liquidity to the pair with some baseTokenAmount. The user lost their asset since they didn't get any lpToken

Proof of Concept

Inside Pair.sol contract, the add() function exist for adding liquidity to the pair. User can input 3 parameters baseTokenAmount, fractionalTokenAmount and minLpTokenAmount and then expect to get an lpToken .

In a scenario where the add() function is called with some amount of baseTokenAmount but the fractionalTokenAmount and minLpTokenAmount is 0, user is still expected to get some lpToken.

Note that, with current implementation, the fractionalTokenAmount is not mandatory to have value, also the minLpTokenAmount, so both can be 0 and the function will not be reverted.

Now if the user input baseTokenAmount some value, while others are 0, then the add() function will not reverted, and unfortunately the user will not be minted any lpToken because the lpTokenAmount will be 0.

Here, user send their baseTokenAmount but didn't get any lpToken, thus lost of asset.

_transferFrom(msg.sender, address(this), fractionalTokenAmount);
lpToken.mint(msg.sender, lpTokenAmount);
if (baseToken != address(0)) {
    ERC20(baseToken).safeTransferFrom(msg.sender, address(this), baseTokenAmount);
}

Tools Used

Manual Analysis

Recommended Mitigation Steps

Check if lpTokenAmount is 0, if so then revert the transaction.

berndartmueller commented 1 year ago

Closing as invalid.

fractionalTokenAmount is asserted to be greater than 0. See https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L71

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid