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

First depositor can break minting of shares #128

Closed
code423n4 opened this issue Dec 16, 2022 · 3 comments
Closed

First depositor can break minting of shares #128

code423n4 opened this issue Dec 16, 2022 · 3 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 upgraded by judge Original issue severity upgraded from QA/Gas by judge

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L421

Vulnerability details

Impact

This is a common vulnerability. The token supply can be manipulated to prevent users from gaining a "normal" number of shares. See Section 3.4 Uniswap does this by burning the first 1000 lpTokens to significantly increase the cost of this attack.

Proof of Concept

First depositor to liquidity pool can deposit a small amount, i.e. 1 wei to mint 1 LP token. They can then donate a large number of baseToken to the protocol and make it difficult for small liquidity providers from minting any LP tokens.

baseTokenShare is calculated as baseTokenAmount * lpTokenSupply / baseTokenReserves(). Because if lpTokenSupply is 1, and if a malicious user donates 100000e18 baseToken, the next depositors need to provide at least 100000e18 baseToken to mint 1 lpToken. If user deposits say 150000e18, they will still only mint 1 lpToken due to the large rounding error.

    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);
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

We can do what Uniswap does, by sending the first 1000 lptoken to the 0 address when liquidity is first initiated.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 16, 2022
code423n4 added a commit that referenced this issue Dec 16, 2022
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #442

@c4-judge
Copy link
Contributor

berndartmueller changed the severity to 3 (High Risk)

@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Jan 10, 2023
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 10, 2023
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 upgraded by judge Original issue severity upgraded from QA/Gas by judge
Projects
None yet
Development

No branches or pull requests

2 participants