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

0x73696d616f - VaultLib::__addLiquidityToAmmUnchecked() does not deal with the remaining amounts not sent to the amm, losing them #236

Closed
sherlock-admin2 opened this issue Sep 10, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Sep 10, 2024

0x73696d616f

High

VaultLib::__addLiquidityToAmmUnchecked() does not deal with the remaining amounts not sent to the amm, losing them

Summary

VaultLib::__addLiquidityToAmmUnchecked() sets a tolerance for the Ra and Ct tokens to provide liquidity, which means it will not revert in case the tokens up to the tolerance are not provided as liquidity to the Amm. However, it does not track the funds that were not provided to the liquidity, leaving them untracked in the Vault.

Root Cause

In VaultLib:45, the remaining Ra and Ct not provided as liquidity are left untracked and are lost.

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

VaultLib::__addLiquidityToAmmUnchecked() is called by VaultLib::__provideLiquidity(), called by VaultLib::__provideLiquidityWithRatio() or __provideAmmLiquidityFromPool(), called when a fee is minted from user actions or a new issuance is started.

Impact

The protocol is left with stuck Ra and Ct.

PoC

VaultLib.sol

function __addLiquidityToAmmUnchecked(
    State storage self,
    uint256 raAmount,
    uint256 ctAmount,
    address raAddress,
    address ctAddress,
    IUniswapV2Router02 ammRouter
) internal {
    (uint256 raTolerance, uint256 ctTolerance) =
        MathHelper.calculateWithTolerance(raAmount, ctAmount, MathHelper.UNIV2_STATIC_TOLERANCE);
    ...
    (,, uint256 lp) = ammRouter.addLiquidity( //@audit remaining amounts are not checked
        token0, token1, token0Amount, token1Amount, token0Tolerance, token1Tolerance, address(this), block.timestamp
    );
    ...
}

Mitigation

Get the actually provided amounts and track the unprovided to liquidity amounts to be dealt with.

Duplicate of #240

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. labels Sep 14, 2024
@sherlock-admin3 sherlock-admin3 changed the title Blurry Blush Mouse - VaultLib::__addLiquidityToAmmUnchecked() does not deal with the remaining amounts not sent to the amm, losing them 0x73696d616f - VaultLib::__addLiquidityToAmmUnchecked() does not deal with the remaining amounts not sent to the amm, losing them Sep 25, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants