code-423n4 / 2024-01-salty-findings

4 stars 3 forks source link

Slippage and Front-Running #1005

Closed c4-bot-5 closed 5 months ago

c4-bot-5 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L83

Vulnerability details

Impact

Detailed description of the impact of this finding. Slippage and Front-Running: The contract does not seem to handle slippage or front-running protection explicitly. Users specify minLiquidityReceived, minReclaimedA, and minReclaimedB to mitigate slippage, but there's no mention of price oracles or other mechanisms to prevent front-running.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

function _depositLiquidityAndIncreaseShare( IERC20 tokenA, IERC20 tokenB, uint256 maxAmountA, uint256 maxAmountB, uint256 minLiquidityReceived, bool useZapping ) internal returns (uint256 addedAmountA, uint256 addedAmountB, uint256 addedLiquidity) { require( exchangeConfig.walletHasAccess(msg.sender), "Sender does not have exchange access" );

    // Transfer the specified maximum amount of tokens from the user
    tokenA.safeTransferFrom(msg.sender, address(this), maxAmountA );
    tokenB.safeTransferFrom(msg.sender, address(this), maxAmountB );

    // Balance the token amounts by swapping one to the other before adding the liquidity?
    if ( useZapping )
        (maxAmountA, maxAmountB) = _dualZapInLiquidity(tokenA, tokenB, maxAmountA, maxAmountB );

    // Approve the liquidity to add
    tokenA.approve( address(pools), maxAmountA );
    tokenB.approve( address(pools), maxAmountB );

    // Deposit the specified liquidity into the Pools contract
    // The added liquidity will be owned by this contract. (external call to Pools contract)
    bytes32 poolID = PoolUtils._poolID( tokenA, tokenB );
    (addedAmountA, addedAmountB, addedLiquidity) = pools.addLiquidity( tokenA, tokenB, maxAmountA, maxAmountB, minLiquidityReceived, totalShares[poolID]);

    // Increase the user's liquidity share by the amount of addedLiquidity.
    // Cooldown is specified to prevent reward hunting (ie - quickly depositing and withdrawing large amounts of liquidity to snipe rewards as they arrive)
    // _increaseUserShare confirms the pool as whitelisted as well.
    _increaseUserShare( msg.sender, poolID, addedLiquidity, true );

Tools Used

Recommended Mitigation Steps

use price oracles for front-running.

Assessed type

Context

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Insufficient proof