The first user of pool gets all of the StakingRewards

[c4-bot-2]

Lines of code

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

Vulnerability details

The first user of pool gets all of the StakingRewards

Impact

The first user of pool gets all of the StakingRewards.

Proof of Concept

StakingRewards#_increaseUserShare if totalShares are 0, user.virtualRewards is not increased:

    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);
            }
        // Update the deposit balances
        user.userShare += uint128(increaseShareAmount);
        totalShares[poolID] = existingTotalShares + increaseShareAmount;
    }

Calculate the userReward need and subtract user.virtualRewards:

    function userRewardForPool( address wallet, bytes32 poolID ) public view returns (uint256)
    {
        .....
        // Determine the share of the rewards for the user based on their deposited share
        uint256 rewardsShare = ( totalRewards[poolID] * user.userShare ) / totalShares[poolID];

        // Reduce by the virtualRewards - as they were only added to keep the share / rewards ratio the same when the used added their share

        // In the event that virtualRewards exceeds rewardsShare due to precision loss - just return zero
        if ( user.virtualRewards > rewardsShare )
            return 0;

        return rewardsShare - user.virtualRewards;
    }

Therefore, if user.virtualRewards is equal to 0, the userRewardForPool function returns an incorrect value.

After testing: If the totalRewards are initialized with 1000 ether and the first user Staking 1 ether, then the first user will get 1000 ether Staking Rewards.

In the DAO, when we create a pool, will add SALTRewards:

    function _finalizeTokenWhitelisting( uint256 ballotID ) internal
        {
        if ( proposals.ballotIsApproved(ballotID ) )
            {
            ......
            // Send the initial bootstrappingRewards to promote initial liquidity on these two newly whitelisted pools
            AddedReward[] memory addedRewards = new AddedReward[](2);
            addedRewards[0] = AddedReward( pool1, bootstrappingRewards );
            addedRewards[1] = AddedReward( pool2, bootstrappingRewards );

            exchangeConfig.salt().approve( address(liquidityRewardsEmitter), bootstrappingRewards * 2 );
            liquidityRewardsEmitter.addSALTRewards( addedRewards );

            emit WhitelistToken(IERC20(ballot.address1));
            }

        // Mark the ballot as finalized (which will also remove it from the list of open token whitelisting proposals)
        proposals.markBallotAsFinalized(ballotID);
        }

Both the Staking module and the Liquidity module call _increaseUserShare function, Staking#stakeSALT Liquidity#depositLiquidityAndIncreaseShare CollateralAndLiquidity#depositCollateralAndIncreaseShare

Test code:

    function testUserRewardsFirst() public {
        AddedReward[] memory addedRewards = new AddedReward[](2);
        addedRewards[0] = AddedReward(poolIDs[0], 1000 ether);
        staking.addSALTRewards(addedRewards);

        vm.prank(alice);
        staking.stakeSALT(1 ether);

        console.log("alice:");
        console.log(staking.userXSalt(alice) /1 ether);
        console.log(staking.userVirtualRewardsForPool(alice, PoolUtils.STAKED_SALT));
        console.log(staking.userRewardForPool(alice, PoolUtils.STAKED_SALT)/1 ether);
    }

Put the test code in Staking.t.sol and run test, we can see from the log that alice's userReward is 1000 ether

[PASS] testUserRewardsFirst() (gas: 172593)
Logs:
  alice:
  1
  0
  1000

Tools Used

vscode, manual

Recommended Mitigation Steps

If existingTotalShares is equal to 0, you also set user.virtualRewards.

Assessed type

Error

[c4-judge]

Picodes marked the issue as duplicate of #614

[c4-judge]

Picodes marked the issue as satisfactory