Once a pool is removed from the whitelist all its pending reward will be permanently stuck

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/rewards/SaltRewards.sol#L117 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/rewards/RewardsEmitter.sol#L57 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/rewards/RewardsEmitter.sol#L82

Vulnerability details

Summary:

Liquidity providers in the protocol are rewarded from various sources, including SALT Emissions, arbitrage profits, and initial bootstrapping rewards. The distribution of these rewards depends on each pool's contribution to generating arbitrage profits. However, an issue arises when a pool is removed from the whitelist through the DAO's decision, leading to the permanent locking of its pending rewards.

Vulnerability Details:

The SaltRewards contract temporarily holds SALT rewards from emissions and arbitrage profits. These rewards are then meant to be distributed to the stakingRewardsEmitter and liquidityRewardsEmitter. The distribution to the latter is based on each pool's share in recent arbitrage profits.

function performUpkeep( bytes32[] calldata poolIDs, uint256[] calldata profitsForPools ) external
        {
         ....
         // Divide up the remaining rewards between SALT stakers and liquidity providers
         uint256 stakingRewardsAmount = ( remainingRewards * rewardsConfig.stakingRewardsPercent() ) / 100;
         uint256 liquidityRewardsAmount = remainingRewards - stakingRewardsAmount;

         _sendStakingRewards(stakingRewardsAmount);
             _sendLiquidityRewards(liquidityRewardsAmount, directRewardsForSaltUSDS, poolIDs, profitsForPools, totalProfits);
        }

In the RewardsEmitter contract, rewards are tracked within the pendingRewards mapping for each pool.

function addSALTRewards(AddedReward[] calldata addedRewards) external nonReentrant {
        ...
            uint256 amountToAdd = addedReward.amountToAdd;
            if (amountToAdd != 0) {
                // Update pendingRewards so the SALT can be distributed later
                pendingRewards[addedReward.poolID] += amountToAdd;
                sum += amountToAdd;
        ...
    }

The performUpkeep function in the RewardsEmitter contract is responsible for the daily distribution of rewards, releasing a percentage (1% per day). The problem is when a pool is removed from the whitelist, it invariably leaves behind a portion of pending rewards within the contract. This remainder is due to the contract's design of emitting only a fixed daily percentage.

Since the performUpkeep function uses the poolsConfig.whitelistedPools function for determining eligible pools for reward distribution. Once a pool is no longer whitelisted, it becomes excluded from this distribution process. The contract also lacks a mechanism to either redistribute these pending rewards to other pools or to transfer them out leaving them locked in the contract permanently.

function performUpkeep(uint256 timeSinceLastUpkeep) external {
        ...

        if (isForCollateralAndLiquidity) {
            // For the liquidityRewardsEmitter, all pools can receive rewards
            poolIDs = poolsConfig.whitelistedPools();

    ...
        // Add the rewards so that they can later be claimed by the users proportional to their share of the StakingRewards derived contract.
        stakingRewards.addSALTRewards(addedRewards);
    }

Impact:

This issue leads to the permanent locking of all pending rewards for any pool removed from the whitelist. These rewards include the remainder of the initial Bootstrapping Rewards, pending SALT Emissions and arbitrage profits.

Tools Used:

• Manual analysis

Recommendation:

Update to the RewardsEmitter contract, introducing a function that either reallocates the pending rewards of delisted pools to active ones or enables their transfer to a specified reserve.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #141

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked the issue as duplicate of #752

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

This previously downgraded issue has been upgraded by Picodes