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

11 stars 6 forks source link

malicious user can exploit the staking reward system #896

Closed c4-bot-2 closed 7 months ago

c4-bot-2 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L78-L86

Vulnerability details

Impact

malicious user increase share right before adding staking salts and decrease share after that and take the rewards .It will be unfair for long term staker .

Proof of Concept

Malicious User deposit wbtc/weth liquidity as collateral to increase share

 function depositCollateralAndIncreaseShare( uint256 maxAmountWBTC, uint256 maxAmountWETH, uint256 minLiquidityReceived, uint256 deadline, bool useZapping ) external nonReentrant ensureNotExpired(deadline)  returns (uint256 addedAmountWBTC, uint256 addedAmountWETH, uint256 addedLiquidity)
    {
    // Have the user deposit the specified WBTC/WETH liquidity and increase their collateral share
    (addedAmountWBTC, addedAmountWETH, addedLiquidity) = _depositLiquidityAndIncreaseShare( wbtc, weth, maxAmountWBTC, maxAmountWETH, minLiquidityReceived, useZapping );

    emit CollateralDeposited(msg.sender, addedAmountWBTC, addedAmountWETH, addedLiquidity);
    }

user will get rewards based on reward/shares ratio

   if ( existingTotalShares != 0 ) // prevent / 0
        {
        // Round up in favor of the protocol.
        uint256 virtualRewardsToAdd = Math.ceilDiv( totalRewards[poolID] * increaseShareAmount, existingTotalShares );

        user.virtualRewards += uint128(virtualRewardsToAdd);
        totalRewards[poolID] += uint128(virtualRewardsToAdd);
        }

    // Update the deposit balances
    user.userShare += uint128(increaseShareAmount);
    totalShares[poolID] = existingTotalShares + increaseShareAmount;

    emit UserShareIncreased(wallet, poolID, increaseShareAmount);
    }

Protocol send rewards to staking rewards contract from reward emitter by calling performUpkeep .Protocol reward system is not based on time .This leads to be exploited .

POC 1.Protocol send rewards to staking rewards contract 2.malicious user see this and they want to increase share before rewards are not reached into staking rewards contract 3.malicious user front run and call deposit collateral & increase share 4.After rewards are now in staking rewards contract ,malicious call withdrawCollateral & claim 5.Malicious only have to wait 1 hr to bypass the check of preventing instant withdraw .

Tools Used

manual view

Recommended Mitigation Steps

make sure user have to wait for long enough to prevent from those attacks

Assessed type

MEV

c4-judge commented 7 months ago

Picodes marked the issue as duplicate of #614

c4-judge commented 6 months ago

Picodes marked the issue as unsatisfactory: Insufficient quality

c4-judge commented 6 months ago

Picodes marked the issue as satisfactory

c4-judge commented 6 months ago

Picodes changed the severity to 3 (High Risk)