stake.link

Summary

The distributeRewards function in the SDLPoolCCIPControllerPrimary contract is vulnerable to inaccurate reward distribution due to unsynchronized reSDLSupplyByChain data. This issue arises when the primary chain's reward distribution does not reflect the latest state of secondary chains, leading to an unequal distribution of rewards.

Vulnerability Details

The flaw occurs in the reward calculation process, where the reSDLSupplyByChain[chainSelector] determines each chain's reward share. If the actual locked amount in a secondary chain is higher than the recorded amount in reSDLSupplyByChain, it leads to that chain receiving fewer rewards. The root cause is the lack of real-time updates to reSDLSupplyByChain, which is only modified during the handleIncomingUpdate function.

PoC

Let's analyze the following scenario:

1. Two secondary chains are added(99, 88).

2. Secondary chain 99 has several locks created.

3. RewardsInitiator triggers the reward distribution, but SDLPoolCCIPControllerPrimary hasn't dyet received an update through _ccipReceive.

4. When distributing the rewards, the reSDLSupplyByChain from SDLPoolCCIPControllerPrimary contains a value that doesn't reflect the new locks added on the secondary chain 99. i.e:
   reSDLSupplyByChain at secondary chain 99: 150
reSDLSupplyByChain at SDLPoolCCIPControllerPrimary: 20

This happens because the reSDLSupplyByChain will only be updated when it receives through CCIP an update on the _ccipReceive function(which has no mechanism to ensure that it will be synced with the distribution of rewards):

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/SDLPoolCCIPControllerPrimary.sol#L82-L85

As there are two chains added, the first chain will have the reward distribution based on: (tokenBalance * reSDLSupplyByChain[chainSelector]) / totalRESDL

Let's formulate with simulated values the result:
tokenBalance = 10
totalRESDL = 10

In the case above, the output is: (10 * 20) / 10 = 20. The chain 99(the first in the loop) will receive 20 tokens as a lower reward distribution than it should. Let's compare the case with the current reSDLSupplyByChain from the SDLPoolSecondary of chain 99:
(10 * 150) / 10 = 150

Impact

This vulnerability can result in significant imbalances in the distribution of rewards, when reSDLSupplyByChain should be a positive value, it will distribute fewer rewards as shown in the PoC above(chain 99 lost 120 tokens as distribution rewards). But when reSDLSupplyByChain is negative and is not synced, the secondary chain will receive more distribution rewards than it should. Over time, it will cause a huge unbalanced distribution of rewards.

Tools Used

Manual Review

Recommendations

As there is no way to ensure a sequence of calls between RewardInitiator(triggered by the Chainlink Keeper) and the sync between secondary chains & primary chain, it is advisable to:

• create a mapping (sourceDestinationChain => bool) shouldDistributeRewards that is set to true when _ccipReceive is called on SDLPoolCCIPControllerPrimary, because it brings the new value of reSDLSupplyByChain for that secondary chain.

• Then on the distributeRewards() function, only the chains that have the shouldDistributeRewards set to true will be the ones receiving the rewards.

• Finally shouldDistributeRewards is set to false when distributeRewards is called for that specific chain.