Disproportionate Virtual Rewards for Early LP Providers

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L57 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L147 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L232 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/Upkeep.sol#L244

Vulnerability details

Vulnerability Details:

Liquidity providers in the protocol earn rewards for their contributions. These rewards are updated each time a user's share changes or they claim rewards. The _increaseUserShare function recalculates and assigns rewards whenever a user increases their stake.

function _increaseUserShare(address wallet, bytes32 poolID, uint256 increaseShareAmount, bool useCooldown)
        internal
    {
        ...
        uint256 existingTotalShares = totalShares[poolID];
        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);
        }
        ...
    }

The protocol uses the virtualRewards variable to account for any rewards before the user, this amount will be deducted when a user claims their rewards or decreases their stake.

Here, virtualRewardsToAdd is computed using the formula:

• virtualRewardsToAdd = (totalRewards[poolID] x increaseShareAmount) x existingTotalShares

This computation can disproportionately affect early LPs, especially when their added amount is comparable to or larger than the existing total shares, leading to inflated virtual rewards and potentially denying them rewards for an extended period.

Example Scenario:

Consider the following:

The protocol can start with up to 5.5k rewards for every LP pool, that is the 1% daily emission. The upkeep function is responsible for transferring up to the maximum allowable daily rewards to the staking contract.

• The upkeep contract employs a timer to regulate the frequency and quantity of rewards distribution. However, since this timer begins counting from the moment of the contract's deployment (in the constructor), and considering the initial voting period for starting up the exchange spans several days, it becomes feasible to distribute the maximum daily reward amount by invoking upkeep.

User 1 adds liquidity, receiving 5 shares.

• User1 won't have virtual rewards calculated, as existingTotalShares would be zero.

User 2 follows, adding 10 shares. The virtualRewardsToAdd for User 2 would be:

• virtualRewardsToAdd = (totalRewards[poolID] * increaseShareAmount) / existingTotalShares
  • (5.5k * 10) / 5 = 11k.
  • This results in User 2 having 11k virtual rewards, despite the pool only containing 5.5k actual rewards.

Assume after several hours, additional users have joined, diluting User 2's share in the pool.

• User2 share = 10
• totalShares = 100
• totalRewards = 5.5k(initial) + user virtual(11k) + other user virtual(80.5k) added rewards(3k) = 100k

User 2 decides to claim their rewards through the claimAllRewards function, which internally utilizes the userRewardForPool function.

• The formula for User 2's reward share calculation:
  • rewardsShare = (totalRewards[poolID] * user.userShare) / totalShares[poolID]
  • (100k * 10) / 100 = 10k

Here, even though the total rewards in the pool have increased to 100k, indicating that rewards have been earned, User 2's share of the rewards (10k) is still less than their inflated virtual rewards total of 11k.

• As a result, the userRewardForPool function will compute a zero reward for User 2, despite the increase in total rewards (from upkeep). This discrepancy can lead to User 2, and potentially other users in similar positions, not receiving their due rewards.

Impact:

Early LPs will have their virtual rewards inflated, leading to a long duration where they cannot claim any rewards.

Proof Of Concept

function testInflatedVirtualRewards() public {
    bytes32 poolID1 = PoolUtils._poolID( wbtc, weth );
    bytes32[] memory poolIDs = new bytes32[](1);
    poolIDs[0] = poolID1;
    skip(2 days);
    uint256 depositedWBTC = ( 1 ether *10**8) / priceAggregator.getPriceBTC();
    uint256 depositedWETH = ( 1 ether *10**18) / priceAggregator.getPriceETH(); 
    (uint256 reserveWBTC, uint256 reserveWETH) = pools.getPoolReserves(wbtc, weth);
    upkeep.performUpkeep();
    // check total rewards of pool
    uint256[] memory rewardsbefore = new uint256[](1);
    rewardsbefore = collateralAndLiquidity.totalRewardsForPools(poolIDs);
    // Alice deposit collateral (first user)
    vm.startPrank(alice);
    collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );
    vm.stopPrank();
    // bob deposit 10 times alice collateral
    _readyUser(bob);
    vm.startPrank(bob);
    collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC * 10, depositedWETH * 10, 0, block.timestamp, false );
    vm.stopPrank();
    // check bobs virtual rewards
    uint virtualRewardsBob = collateralAndLiquidity.userVirtualRewardsForPool(bob, poolIDs[0]);
    // bobs virtual rewards is greater then all initial pool rewards
    assertTrue(virtualRewardsBob > rewardsbefore[0]);
    }

Tools Used:

• Manual analysis
• Foundry

Recommendation:

Invoking the performUpkeep function just before the initial distribution can help mitigate this issue. This action resets the timer, ensuring that rewards do not start at the maximum daily amount, thus preventing the overinflation of virtual rewards for early LPs.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #614

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 3 (High Risk)