Salty.IO - 0xbepresent's results

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L140 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L70

Vulnerability details

Impact

The CollateralAndLiquidity contract contains a critical vulnerability that allows a user undergoing liquidation to evade the process by manipulating the user.cooldownExpiration variable. This manipulation is achieved through the CollateralAndLiquidity::depositCollateralAndIncreaseShare function, specifically within the StakingRewards::_increaseUserShare function (code line #70):

File: StakingRewards.sol
57: 	function _increaseUserShare( address wallet, bytes32 poolID, uint256 increaseShareAmount, bool useCooldown ) internal
58: 		{
59: 		require( poolsConfig.isWhitelisted( poolID ), "Invalid pool" );
60: 		require( increaseShareAmount != 0, "Cannot increase zero share" );
61: 
62: 		UserShareInfo storage user = _userShareInfo[wallet][poolID];
63: 
64: 		if ( useCooldown )
65: 		if ( msg.sender != address(exchangeConfig.dao()) ) // DAO doesn't use the cooldown
66: 			{
67: 			require( block.timestamp >= user.cooldownExpiration, "Must wait for the cooldown to expire" );
68: 
69: 			// Update the cooldown expiration for future transactions
70: 			user.cooldownExpiration = block.timestamp + stakingConfig.modificationCooldown();
71: 			}
72: 
73: 		uint256 existingTotalShares = totalShares[poolID];
74: 
75: 		// Determine the amount of virtualRewards to add based on the current ratio of rewards/shares.
76: 		// The ratio of virtualRewards/increaseShareAmount is the same as totalRewards/totalShares for the pool.
77: 		// The virtual rewards will be deducted later when calculating the user's owed rewards.
78:         if ( existingTotalShares != 0 ) // prevent / 0
79:         	{
80: 			// Round up in favor of the protocol.
81: 			uint256 virtualRewardsToAdd = Math.ceilDiv( totalRewards[poolID] * increaseShareAmount, existingTotalShares );
82: 
83: 			user.virtualRewards += uint128(virtualRewardsToAdd);
84: 	        totalRewards[poolID] += uint128(virtualRewardsToAdd);
85: 	        }
86: 
87: 		// Update the deposit balances
88: 		user.userShare += uint128(increaseShareAmount);
89: 		totalShares[poolID] = existingTotalShares + increaseShareAmount;
90: 
91: 		emit UserShareIncreased(wallet, poolID, increaseShareAmount);
92: 		}

Malicious user can perform front-running of the liquidation function by depositing small amounts of tokens to his position, incrementing the user.cooldownExpiration variable. Consequently, the execution of the liquidation function will be reverted with the error message Must wait for the cooldown to expire. This vulnerability could lead to attackers evading liquidation, potentially causing the system to enter into debt as liquidations are avoided.

Proof of Concept

A test case, named testUserLiquidationMayBeAvoided, has been created to demonstrate the potential misuse of the system. The test involves the following steps:

1. User Alice deposits and borrow the maximum amount.
2. The collateral price crashes.
3. Alice maliciously front-runs the liquidation execution by depositing a the minimum amount using the collateralAndLiquidity::depositCollateralAndIncreaseShare function.
4. The liquidation transaction is reverted by "Must wait for the cooldown to expire" error.

// Filename: src/stable/tests/CollateralAndLiquidity.t.sol:TestCollateral
// $ forge test --match-test "testUserLiquidationMayBeAvoided" --rpc-url https://yoururl -vv
//
    function testUserLiquidationMayBeAvoided() public {
        // Liquidatable user can avoid liquidation
        //
		// Have bob deposit so alice can withdraw everything without DUST reserves restriction
        _depositHalfCollateralAndBorrowMax(bob);
        //
        // 1. Alice deposit and borrow the max amount
        // Deposit and borrow for Alice
        _depositHalfCollateralAndBorrowMax(alice);
        // Check if Alice has a position
        assertTrue(_userHasCollateral(alice));
        //
        // 2. Crash the collateral price
        _crashCollateralPrice();
        vm.warp( block.timestamp + 1 days );
        //
        // 3. Alice maliciously front run the liquidation action and deposit a DUST amount
        vm.prank(alice);
		collateralAndLiquidity.depositCollateralAndIncreaseShare(PoolUtils.DUST + 1, PoolUtils.DUST + 1, 0, block.timestamp, false );
        //
        // 4. The function alice liquidation will be reverted by "Must wait for the cooldown to expire"
        vm.expectRevert( "Must wait for the cooldown to expire" );
        collateralAndLiquidity.liquidateUser(alice);
    }

Tools used

Manual review

Recommended Mitigation Steps

Consider modifying the liquidation function as follows:

	function liquidateUser( address wallet ) external nonReentrant
		{
		require( wallet != msg.sender, "Cannot liquidate self" );

		// First, make sure that the user's collateral ratio is below the required level
		require( canUserBeLiquidated(wallet), "User cannot be liquidated" );

		uint256 userCollateralAmount = userShareForPool( wallet, collateralPoolID );

		// Withdraw the liquidated collateral from the liquidity pool.
		// The liquidity is owned by this contract so when it is withdrawn it will be reclaimed by this contract.
		(uint256 reclaimedWBTC, uint256 reclaimedWETH) = pools.removeLiquidity(wbtc, weth, userCollateralAmount, 0, 0, totalShares[collateralPoolID] );

		// Decrease the user's share of collateral as it has been liquidated and they no longer have it.
--		_decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true );
++		 _decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, false );

		// The caller receives a default 5% of the value of the liquidated collateral.
		uint256 rewardPercent = stableConfig.rewardPercentForCallingLiquidation();

		uint256 rewardedWBTC = (reclaimedWBTC * rewardPercent) / 100;
		uint256 rewardedWETH = (reclaimedWETH * rewardPercent) / 100;

		// Make sure the value of the rewardAmount is not excessive
		uint256 rewardValue = underlyingTokenValueInUSD( rewardedWBTC, rewardedWETH ); // in 18 decimals
		uint256 maxRewardValue = stableConfig.maxRewardValueForCallingLiquidation(); // 18 decimals
		if ( rewardValue > maxRewardValue )
			{
			rewardedWBTC = (rewardedWBTC * maxRewardValue) / rewardValue;
			rewardedWETH = (rewardedWETH * maxRewardValue) / rewardValue;
			}

		// Reward the caller
		wbtc.safeTransfer( msg.sender, rewardedWBTC );
		weth.safeTransfer( msg.sender, rewardedWETH );

		// Send the remaining WBTC and WETH to the Liquidizer contract so that the tokens can be converted to USDS and burned (on Liquidizer.performUpkeep)
		wbtc.safeTransfer( address(liquidizer), reclaimedWBTC - rewardedWBTC );
		weth.safeTransfer( address(liquidizer), reclaimedWETH - rewardedWETH );

		// Have the Liquidizer contract remember the amount of USDS that will need to be burned.
		uint256 originallyBorrowedUSDS = usdsBorrowedByUsers[wallet];
		liquidizer.incrementBurnableUSDS(originallyBorrowedUSDS);

		// Clear the borrowedUSDS for the user who was liquidated so that they can simply keep the USDS they previously borrowed.
		usdsBorrowedByUsers[wallet] = 0;
		_walletsWithBorrowedUSDS.remove(wallet);

		emit Liquidation(msg.sender, wallet, reclaimedWBTC, reclaimedWETH, originallyBorrowedUSDS);
		}

This modification ensures that the user.cooldownExpiration expiration check does not interfere with the liquidation process, mitigating the identified security risk.

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/rewards/RewardsEmitter.sol#L82 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolsConfig.sol#L63

Vulnerability details

Impact

The RewardsEmitter.sol contract is designed to store rewards (SALT tokens) assigned to whitelisted pools, then these rewards are gradually distributed to StakingRewards.sol at a rate of 1% per day using the RewardsEmitter::performUpkeep function. The execution flow for assigning rewards from RewardsEmitter.sol to StakingRewards.sol:

1. The Emissions.sol contract or deposited balance in Upkeep.sol is transferred to SaltRewards.
2. Subsequently, during the system update in Upkeep::step7, the SaltRewards::performUpkeep function dispatches rewards to stakingRewardsEmitter and liquidityRewardsEmitter.
3. In step Upkeep::step8, these rewards are distributed at a daily rate to the respective pools using the RewardsEmitter::performUpkeep function.
4. Finally, the rewards reach StakingRewards for the designated pool through the StakingRewards::addSALTRewards function.

The issue arises when rewards are assigned to a whitelisted pool, and that pool is later removed from the whitelist using the PoolsConfig::unwhitelistPool function. This removal leaves the assigned rewards stranded in the RewardsEmitter without being distributed to the stakingRewards because the RewardsEmitter::performUpkeep process only the whitelisted pools excluding the rewards assigned to the removed pools.

Proof of Concept

The following test demonstrates the problem. Initially, 10 ether in rewards is assigned to poolIDs[0]. Subsequently, poolIDs[0] is removed using the PoolsConfig::unwhitelistPool function. The test then verifies that the rewards assigned to the removed pool (poolIDs[0]) remain in the RewardsEmitter, in the other hand, the remaining whitelisted pool poolIDs[1] rewards amount is deducted correctly:

1. Add some pending rewards to the pools.
2. Verify that the rewards were added.
3. Whitelisted pool (poolIds[0]) is removed.
4. Call performUpkeep.
5. Verify that the correct amount of rewards were deducted only to the whitelisted pool (poolIds[1]). The rewards assigned to the removed pool (poolIDs[0]) remains in RewardsEmitter.

// Filename: src/rewards/tests/RewardsEmitter.t.sol:TestRewardsEmitter
// $ forge test --match-test "testRewardsToMultiplePoolsThenOnePoolIsRemoved" --rpc-url https://yourrpc.com -vv
//
    function testRewardsToMultiplePoolsThenOnePoolIsRemoved() public {
        // Whitelisted pool may be removed even when there are rewards assigned to that pool
        //
        // 1. Add some pending rewards to the pools
        AddedReward[] memory addedRewards = new AddedReward[](2);
        addedRewards[0] = AddedReward({poolID: poolIDs[0], amountToAdd: 10 ether});
        addedRewards[1] = AddedReward({poolID: poolIDs[1], amountToAdd: 10 ether});
        liquidityRewardsEmitter.addSALTRewards(addedRewards);
        //
        // 2. Verify that the rewards were added
        assertEq(pendingLiquidityRewardsForPool(poolIDs[0]), 10 ether);
        assertEq(pendingLiquidityRewardsForPool(poolIDs[1]), 10 ether);
        //
        // 3. Whitelisted pool (poolIds[0]) is removed
        vm.prank(address(dao));
        poolsConfig.unwhitelistPool( pools, token1, token2);
        //
        // 4. Call performUpkeep
        vm.prank(address(upkeep));
        liquidityRewardsEmitter.performUpkeep(1 days);
        //
        // 5. Verify that the correct amount of rewards were deducted only to the whitelisted pool (poolIds[1]). 
        // The rewards assigned to the removed pool (poolIDs[0]) remains in `RewardsEmitter`.
        assertEq(pendingLiquidityRewardsForPool(poolIDs[0]), 10 ether); // 10 ether. Rewards were not deducted
        assertEq(pendingLiquidityRewardsForPool(poolIDs[1]), 9.75 ether); // 10 ether - 2.5%
    }

Tools used

Manual review

Recommended Mitigation Steps

Upon removal of a pool using the PoolsConfig::unwhitelistPool function, the rewards assigned to the removed pool should be redistributed to the remaining whitelisted pools. This ensures that no rewards are left unclaimed in the RewardsEmitter and maintains fair distribution among active pools.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolStats.sol#L37 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolStats.sol#L134 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolStats.sol#L47

Vulnerability details

Impact

The Upkeep::performUpkeep function, in step 2, withdraws profits generated from arbitrage within the pools. These rewards are then distributed among the involved pools. The system accumulates arbitrage profits for each pool in the PoolStats::_updateProfitsFromArbitrage function. The execution process is as follows:

1. Upkeep::perfomUpkeep is called, and it invokes dao::withdrawArbitrageProfits.
2. The dao::withdrawArbitrageProfits function retrieves the arbitrage generated for the DAO in wETH.
3. Subsequently, wETH is distributed among the POLs, and the remaining wETH is sent to the SaltRewards and then it distributes to the pools that contributed to arbitrage through the SaltRewards::performUpkeep function.

The issue arises when a pool that has generated arbitrage profits is deactivated using the poolsConfig::unwhitelistPool function. This causes the arbitrage-generated wETH from the unwhitelisted pool to be sent to SaltRewards but without delivering those arbitrage profits to the pool since the pool is no longer active. Additionally, the PoolStats::_arbitrageProfits variable of the deactivated pool is not cleared, causing the system to continue counting arbitrage profits that have already been sent to SaltRewards.

Considering the above, let's examine the following scenario:

1. Suppose poolAB generates 5 wETH, and poolBC generates 5 wETH as arbitrage profit.
2. poolBC is unwhitelisted.
3. Upkeep::perfomUpkeep is called, invoking dao::withdrawArbitrageProfits, obtaining the total 10e18 wETH tokens, including the profits from the deactivated pool (poolBC).
4. The 10e18 wETH tokens are distributed among the POLs, and the remainder is sent to SaltRewards to the whitelisted pools. In SaltRewards::performUpkeep, it is distributed only to the whitelisted poolAB, leaving the rest wETH in the SaltRewards contract.

Therefore, in the given example, the PoolStats::_arbitrageProfits variable of the deactivated pool (poolBC) remains unchanged. However, these profits have already been distributed among the POLs and SaltRewards to the whitelisted pools. Additionally, if the deactivated pool is reactivated, the PoolStats::_arbitrageProfits variable will have a non-zero value, causing it to count arbitrage profits that were previously delivered to SaltRewards.

This demonstrates that even though those profits from the unwhitelisted pool have already been distributed improperly to SaltRewards, the system will still take into account that the inactive pool has arbitrage profits, which can be detrimental if that pool becomes active again.

Proof of Concept

A test scenario was created to illustrate the issue. In the test, the poolIDBC is assigned 30e18 as profits. Subsequently, poolIDBC is deactivated, and the PoolStats::clearProfitsForPools function does not clear PoolStats::_arbitrageProfits for poolBC, leaving it unchanged at 30e18.

1. Setup test. Whitelist pools, deposit some profits from arbitrage, calculate and verify the expected profits per pool .
2. Unwhitelist the tokenB/tokenC pool.
3. Simulate the profits are sent to the pools. arbitrageProfits for poolIDAB is empty because those rewards were sent to the poolIDAB.
4. poolIDBC still has arbitrageProfits=30e18 which is incorrectly because dao::withdrawArbitrageProfits will extract all wETH arbitrage profits.

// Filename: src/pools/tests/PoolStats.t.sol:TestPoolStats
// $ forge test --rpc-url https://yourrpc.com --match-test="testProfitsForPoolsThenUnWhitelistAPool" -vvv
//
	function testProfitsForPoolsThenUnWhitelistAPool() public {
		//
		// 1. Setup test. Whitelist pools, deposit some profits from arbitrage, calculate and verify the expected profits per pool 
		// Whitelist pools
		vm.startPrank(address(deployment.dao()));
		deployment.poolsConfig().whitelistPool(deployment.pools(), tokenA, tokenB);
		deployment.poolsConfig().whitelistPool(deployment.pools(), tokenB, tokenC);
		vm.stopPrank();
		// Update arbitrage indicies
		this.updateArbitrageIndicies();
		// Set arbitrage profits
		this.updateProfitsFromArbitrage(tokenA, tokenB, 20 ether);
		this.updateProfitsFromArbitrage(tokenB, tokenC, 30 ether);
		// Get profits for whitelisted pools
		uint256[] memory profits = this.profitsForWhitelistedPools();
		// Fetch the pool IDs
		bytes32 poolIDAB = PoolUtils._poolID(tokenA, tokenB);
		bytes32 poolIDBC = PoolUtils._poolID(tokenB, tokenC);
		// Fetch the pool indexes
		bytes32[] memory whitelistedPools = poolsConfig.whitelistedPools();
		uint64 indexAB = _poolIndex(tokenA, tokenB, whitelistedPools);
		uint64 indexBC = _poolIndex(tokenB, tokenC, whitelistedPools);
		// Calculate expected profits distributed per pool
		uint256 expectedProfitPerPoolAB = _arbitrageProfits[poolIDAB] / 3;
		uint256 expectedProfitPerPoolBC = _arbitrageProfits[poolIDBC] / 3;
		assertEq(_arbitrageProfits[poolIDBC], 30e18);
		// Verify that each pool has received the correct share of profits
		assertEq(profits[indexAB], expectedProfitPerPoolAB, "Pool tokenA/tokenB received incorrect share of profits");
		assertEq(profits[indexBC], expectedProfitPerPoolBC, "Pool tokenB/tokenC received incorrect share of profits");
		//
		// 2. Unwhitelist the tokenB/tokenC pool
		vm.startPrank(address(deployment.dao()));
		deployment.poolsConfig().unwhitelistPool(deployment.pools(), tokenB, tokenC);
		vm.stopPrank();
		//
		// 3. Simulate the profits are sent to the pools. `arbitrageProfits` for poolIDAB is empty, those rewards were sent to the poolIDAB
		vm.prank(address(deployment.upkeep()));
		this.clearProfitsForPools();
		profits = this.profitsForWhitelistedPools();
		assertEq(_arbitrageProfits[poolIDAB], 0);
		assertEq(profits[indexAB], 0);
		//
		// 4. `poolIDBC` still has `arbitrageProfits=30e18` but `profits` does not have the `indexBC` index (index out of bounds).
		assertEq(_arbitrageProfits[poolIDBC], 30e18);
		vm.expectRevert();
		profits[indexBC];
	}

Tools used

Manual review

Recommended Mitigation Steps

Before removing a pool, it is essential to send the arbitrage profits generated by that pool to SaltRewards to ensure that PoolStats::_arbitrageProfits is set to zero when clearProfitsForPools is called. This step will prevent the counting of arbitrage profits that have already been distributed among the POLs and SaltRewards. Implementing this mitigation step will maintain the integrity of arbitrage profit accounting within the system.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/rewards/Emissions.sol#L40 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/Upkeep.sol#L187

Vulnerability details

Impact

The Emissions::performUpkeep function is responsible for distributing SALT tokens at a rate of 0.50% per week to the SaltRewards contract. This rate is specified in the documentation:

Emissions.sol handles distribution of the 52 million SALT stored in the contract at the default rate of .50% per week. The emissions are distributed on performUpkeep() directly to SaltRewards.sol.

In the other hand, the Upkeep::performUpkeep function calls Emissions::performUpkeep in step 6; hence, the counting of days since the last time performUpkeep was invoked is carried out in the Upkeep.sol contract.

However, there is a limitation in the code that can prevent rewards from being distributed at the intended rate. The validation in the Emissions::performUpkeep function restricts the number of days for which rewards are distributed, capping it at one week, as indicated in the documentation. This limitation may result in rewards not being distributed according to the specified rate of 0.50% per week (code line L47-L50):

File: Emissions.sol
40: 	function performUpkeep(uint256 timeSinceLastUpkeep) external
41: 		{
...
...
46: 
47: 		// Cap the timeSinceLastUpkeep at one week (if for some reason it has been longer).
48: 		// This will cap the emitted rewards at a default of 0.50% in this transaction.
49: 		if ( timeSinceLastUpkeep >= MAX_TIME_SINCE_LAST_UPKEEP )
50: 			timeSinceLastUpkeep = MAX_TIME_SINCE_LAST_UPKEEP;
51: 
...
...

Proof of Concept

A test scenario was created to illustrate the issue. In the test, three weeks have passed, but the SaltRewards contract has only received rewards equivalent to two weeks. This discrepancy demonstrates that Emissions is not distributing rewards at the expected rate of 0.50% per week. Test steps:

1. Time goes by 3 days and salt rewards receives rewards equivalent to 3 days.
2. Time goes by 2 weeks. Salt rewards receives rewards equivalent to 7 days because there is a cap to 1 week.
3. Time goes by 4 days. Salt rewards receives rewards equivalent to 4 days.
4. SaltRewards has less rewards than it should be. It has been passed 3 weeks (3 days + 2 weeks + 4 days). For 3 weeks, it should receive approx 15e18 tokens but SaltRewards has received only 10e18 approx.

// Filename: src/rewards/tests/Emissions.t.sol:TestEmissions
// $ forge test --rpc-url https://yourrpc.com --match-test="testPerformUpkeepMaxTimeRebase"
//
		function testPerformUpkeepMaxTimeRebase() public {
			assertTrue(salt.balanceOf(address(saltRewards)) == 0);
			uint256 saltBalance = salt.balanceOf(address(emissions));
			uint256 initialSaltBalance = saltBalance;
			uint256 lastUpKeep = block.timestamp;
			console.log("Emissions initial balance:       ", initialSaltBalance); //1000e18
			//
			// 1. Time goes by 3 days and salt rewards receives rewards equivalent to 3 days.
			vm.warp(block.timestamp + 3 days);
			vm.prank(address(upkeep));
			uint256 timeSinceLastUpkeep = block.timestamp - lastUpKeep;
			emissions.performUpkeep(timeSinceLastUpkeep);
			lastUpKeep = block.timestamp;
			uint256 expectedSaltSent = ( saltBalance * timeSinceLastUpkeep * rewardsConfig.emissionsWeeklyPercentTimes1000() ) / ( 100 * 1000 weeks );
			assertEq(saltBalance - salt.balanceOf(address(emissions)), expectedSaltSent);
			console.log("Salt rewards for 3 days:         ", expectedSaltSent);
			saltBalance = salt.balanceOf(address(emissions));
			//
			// 2. Time goes by 2 weeks. Salt rewards receives rewards equivalent to 7 days because there is a cap to 1 week.
			vm.warp(block.timestamp + 2 weeks);
			vm.prank(address(upkeep));
			timeSinceLastUpkeep = block.timestamp - lastUpKeep;
			emissions.performUpkeep(timeSinceLastUpkeep);
			lastUpKeep = block.timestamp;
			// capped to 1 week
			expectedSaltSent = ( saltBalance * MAX_TIME_SINCE_LAST_UPKEEP * rewardsConfig.emissionsWeeklyPercentTimes1000() ) / ( 100 * 1000 weeks );
			assertEq(saltBalance - salt.balanceOf(address(emissions)), expectedSaltSent);
			console.log("Salt rewards for 7 days (capped but in reality has be passed 2 weeks): ", expectedSaltSent);
			saltBalance = salt.balanceOf(address(emissions));
			//
			// 3. Time goes by 4 days. Salt rewards receives rewards equivalent to 4 days.
			vm.warp(block.timestamp + 4 days);
			vm.prank(address(upkeep));
			timeSinceLastUpkeep = block.timestamp - lastUpKeep;
			emissions.performUpkeep(timeSinceLastUpkeep);
			lastUpKeep = block.timestamp;
			expectedSaltSent = ( saltBalance * timeSinceLastUpkeep * rewardsConfig.emissionsWeeklyPercentTimes1000() ) / ( 100 * 1000 weeks );
			assertEq(saltBalance - salt.balanceOf(address(emissions)), expectedSaltSent);
			console.log("Salt rewards for 4 days: ", expectedSaltSent);
			saltBalance = salt.balanceOf(address(emissions));
			//
			// 4. `SaltRewards` has less rewards than it should be. Emissions should emmit 0.50% per week, it has been passed 3 weeks (3 days + 2 weeks + 4 days) 
			// For 3 weeks, it should receive approx `15e18` tokens but SaltRewards has received `10e18` approx
			uint256 expectedTotalSaltSent = ( initialSaltBalance * (3 weeks) * rewardsConfig.emissionsWeeklyPercentTimes1000() ) / ( 100 * 1000 weeks );
			assertLt(salt.balanceOf(address(saltRewards)), expectedTotalSaltSent - 5e18);
			console.log("Expected salt rewards for 3 weeks:", expectedTotalSaltSent);
			console.log("Current salt rewards balance:     ", salt.balanceOf(address(saltRewards)));
		}

Test log:

[PASS] testPerformUpkeepMaxTimeRebase() (gas: 103611)
Logs:
  Emissions initial balance:        1000000000000000000000
  Salt rewards for 3 days:          2142857142857142857
  Salt rewards for 7 days (capped but in reality has be passed 2 weeks):  4989285714285714285
  Salt rewards for 4 days:  2836765306122448979
  Expected salt rewards for 3 weeks: 15000000000000000000
  Current salt rewards balance:      9968908163265306121

The expected rewards for 3 weeks should be an appox of 15e18, but the SaltRewards receives only 10e18 approx.

Tools used

Manual review

Recommended Mitigation Steps

To address this issue, it is recommended to remove the limitation on rewards distribution to a maximum of seven days. This limitation can prevent the correct distribution of rewards if the Upkeep::performUpkeep function has not been executed for more than seven days. If, for example, Upkeep::performUpkeep has not been executed for three weeks, Emissions should distribute rewards equivalent to the elapsed three weeks. Removing the cap ensures that rewards are distributed at the correct rate based on the actual time that has passed since the last upkeep.

Assessed type

Context