stake.link

Summary

The CCIP documentation states that "This function(ccipReceive) should never revert, all errors should be handled internally in this contract." It also states that this is because CCIP doesn't revert.(https://docs.chain.link/ccip/tutorials/programmable-token-transfers-defensive)
However, the protocol performs complex processing in the ccipReceive function, and there are a great many parts that can be revert.
In this report, I will list as examples those areas where I believe a revert is realistically possible.

Vulnerability Details

1. If fees is exceeded

    if (fees > maxLINKFee) revert FeeExceedsLimit(fees);

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

This is a normal occurrence depending on the maxLINKFee setting value.
If an error occurs here, ccipReceive will fail because of the process in_ccipSendUpdate, which is called by the _ccipReceive function.

2. if ccipSend failed

    bytes32 messageId = router.ccipSend(_destinationChainSelector, evm2AnyMessage);

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

ccipSend fails for a variety of reasons.(https://docs.chain.link/ccip/api-reference/errors)
If an error occurs here, ccipReceive will fail because of the process in _ccipSendUpdate, which is called by the _ccipReceive function.
Because of the possibility of failure due to external factors, ccipSend should be able to supplement errors with try/catch and then resend.

3. If whitelistedDestinations[sourceChainSelector] is deleted

    if (sender != whitelistedDestinations[sourceChainSelector]) revert SenderNotAuthorized();

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

With removeWhitelistedChain, the owner can remove the whitelistedDestinations[sourceChainSelector] at any time.
In other words, ccipReceive will fail if it is removed at a time when messages are being exchanged.
This may seem like a rare case, but assuming that the protocol is active and CCIP messages are constantly being exchanged, it is quite possible.
It might be a good idea to introduce a pause function in the protocol that would stop the exchange of CCIP messages.

4. Errors during token distribution in the secondary chain

            ISDLPoolSecondary(sdlPool).distributeTokens(rewardTokens);

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/SDLPoolCCIPControllerSecondary.sol#L156C17-L156C75

Within the _ccipReceive function, distributeTokens is executed.
The processing inside distributeTokens is outside the scope of the audit, but if a token scheduled for distribution is removed from the whitelist, it is reverted.
This in itself does not seem likely, but since distributeTokens handles the distribution of multiple tokens at once, _ccipReceive will revert if an anomaly occurs with any one token.
The situation is serious because an error in one token will also prevent the distribution of rewards for other tokens.

In addition to the above, there are other places where revert is described and other processes where revert may occur. Although not described because no realistic pattern could be found, all possible error cases must be handled appropriately.

Impact

If ccipSend worked but ccipReceive failed. Improper state handling will have occurred, and the chain will not be consistent with each other, making the protocol fundamentally unworkable.

Tools Used

Manual

Recommendations

Error handling should be done so that it never reverts.