Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A malicious early user/attacker can manipulate the lpToken's pricePerShare to take an unfair share of future users' deposits #470

Closed
code423n4 opened this issue Dec 19, 2022 · 2 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-442 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L275-L286
https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L63-L100
https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L417-L428

Vulnerability details

Impact

A well known attack vector for almost all shares based liquidity pool contracts, where an early user can manipulate the price per share and profit from late users' deposits because of the precision loss caused by the rather large value of price per share..

Proof of Concept

Attacker add 1 nft and can receive 1e18 value of fractionalTokenAmount at line 282 uint256 fractionalTokenAmount = wrap(tokenIds, proofs);

function nftAdd(
    uint256 baseTokenAmount,
    uint256[] calldata tokenIds,
    uint256 minLpTokenAmount,
    bytes32[][] calldata proofs
) public payable returns (uint256 lpTokenAmount) {
    // wrap the incoming NFTs into fractional tokens
    uint256 fractionalTokenAmount = wrap(tokenIds, proofs);


    // add liquidity using the fractional tokens and base tokens
    lpTokenAmount = add(baseTokenAmount, fractionalTokenAmount, minLpTokenAmount);
}

Now, at line 285, add function is called with baseTokenAmount = 1e18, fractionalTokenAmount = 1e18, and minLpTokenAmount is some x amount.

function add(uint256 baseTokenAmount, uint256 fractionalTokenAmount, uint256 minLpTokenAmount)
    public
    payable
    returns (uint256 lpTokenAmount)
{
    // *** Checks *** //


    // check the token amount inputs are not zero
    require(baseTokenAmount > 0 && fractionalTokenAmount > 0, "Input token amount is zero");


    // check that correct eth input was sent - if the baseToken equals address(0) then native ETH is used
    require(baseToken == address(0) ? msg.value == baseTokenAmount : msg.value == 0, "Invalid ether input");


    // calculate the lp token shares to mint
    lpTokenAmount = addQuote(baseTokenAmount, fractionalTokenAmount);


    // check that the amount of lp tokens outputted is greater than the min amount
    require(lpTokenAmount >= minLpTokenAmount, "Slippage: lp token amount out");


    // *** Effects *** //


    // transfer fractional tokens in
    _transferFrom(msg.sender, address(this), fractionalTokenAmount);


    // *** Interactions *** //


    // mint lp tokens to sender
    lpToken.mint(msg.sender, lpTokenAmount);


    // transfer base tokens in if the base token is not ETH
    if (baseToken != address(0)) {
        // transfer base tokens in
        ERC20(baseToken).safeTransferFrom(msg.sender, address(this), baseTokenAmount);
    }


    emit Add(baseTokenAmount, fractionalTokenAmount, lpTokenAmount);
}

In add function, at line 77, addQuote is called. lpTokenAmount = addQuote(baseTokenAmount, fractionalTokenAmount);

function addQuote(uint256 baseTokenAmount, uint256 fractionalTokenAmount) public view returns (uint256) {
    uint256 lpTokenSupply = lpToken.totalSupply();
    if (lpTokenSupply > 0) {
        // calculate amount of lp tokens as a fraction of existing reserves
        uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();
        uint256 fractionalTokenShare = (fractionalTokenAmount * lpTokenSupply) / fractionalTokenReserves();
        return Math.min(baseTokenShare, fractionalTokenShare);
    } else {
        // if there is no liquidity then init
        return Math.sqrt(baseTokenAmount * fractionalTokenAmount);
    }
}

since it is first deosit, the lpTokenSupply will be zero, so else part is executed and the return Math.sqrt(baseTokenAmount * fractionalTokenAmount);

The total amount of lptoken is 1e18.

Then the attacker can send 10000e18 - 1 of baseToken tokens and inflate the price per share from 1.0000 to an extreme value of 1.0000e22 ( from (1 + 10000e18 - 1) / 1) .

As a result, the future user who deposits 19999e18 will only receive 1 wei (from 19999e18 * 1 / 10000e18) of shares token.

They will immediately lose 9999e18 or half of their deposits if they sell and liquidate right after the deposit().

Tools Used

Manual review

Recommended Mitigation Steps

Consider requiring a minimal amount of share tokens to be minted for the first minter, and send a port of the initial mints as a reserve to the DAO so that the pricePerShare can be more resistant to manipulation.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 19, 2022
code423n4 added a commit that referenced this issue Dec 19, 2022
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #442

C4-Staff added a commit that referenced this issue Jan 6, 2023
@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 10, 2023
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-442 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants