sherlock-audit / 2024-08-cork-protocol-judging

2 stars 2 forks source link

0xEkko - Repurchase Function Incorrectly Uses `fee` Instead of `amount` for Liquidity Provision, Leading to Reduced Pool Liquidity #170

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

0xEkko

High

Repurchase Function Incorrectly Uses fee Instead of amount for Liquidity Provision, Leading to Reduced Pool Liquidity

Summary

In the repurchase function in PsmLib.sol, the calculated fee is incorrectly passed to provideLiquidityWithFee in VaultLib.sol as the amount for liquidity provision, instead of the full amount of Redemption Asset (RA). This results in unintended behavior, as the function is designed to handle the total RA amount intended for liquidity, not just the fee. This can reduce the overall liquidity in the pool and negatively impact the protocol’s efficiency.

Vulnerability Detail

The repurchase() function calculates a fee during the repurchase process. This fee is mistakenly passed to the provideLiquidityWithFee() function as the amount parameter. However, the subsequent function, __provideLiquidityWithRatio(), expects this amount to represent the total Redemption Asset (RA) available for liquidity provision. Since the fee is significantly smaller than the full RA amount, this leads to insufficient liquidity being added to the pool, disrupting the protocol’s liquidity balance and efficiency.

Impact

Using the fee as the liquidity provision amount can result in insufficient liquidity being provided to the Automated Market Maker (AMM), potentially affecting the stability and efficiency of the liquidity pool. This could lead to reduced liquidity depth, increased slippage for trades, and potential financial losses for liquidity providers.

Code Snippet

    function repurchase(
        State storage self,
        address buyer,
        uint256 amount,
        IDsFlashSwapCore flashSwapRouter,
        IUniswapV2Router02 ammRouter
    ) internal returns (uint256 dsId, uint256 received, uint256 feePrecentage, uint256 fee, uint256 exchangeRates) {
        DepegSwap storage ds;

@>  (dsId, received, feePrecentage, fee, exchangeRates, ds) = previewRepurchase(self, amount);

        // decrease PSM balance
        // we also include the fee here to separate the accumulated fee from the repurchase
        self.psm.balances.paBalance -= (received);
        self.psm.balances.dsBalance -= (received);

        // transfer user RA to the PSM/LV
        self.psm.balances.ra.lockUnchecked(amount, buyer);

        // transfer user attrubuted DS + PA
        // PA
        (, address pa) = self.info.underlyingAsset();
        IERC20(pa).safeTransfer(buyer, received);

        // DS
        IERC20(ds._address).transfer(buyer, received);

        // Provide liquidity
@> VaultLibrary.provideLiquidityWithFee(self, fee, flashSwapRouter, ammRouter);
    }

Tool used

Manual Review

Recommendation

Ensure that the correct amount (total RA intended for liquidity) is passed to the function, rather than just the fee.

sherlock-admin3 commented 1 month ago

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

This is exactly the purpose of the function: to pass the fees and not the full amount.