Tokens distribution may be broken due to incorrect address verification when depositing tokens

Summary

According to the documentation the YsDistributor.depositMultipleToken function is only usable by this multisig and will update the amount of ERC20 tokens available as rewards for the Locking position holding ysCvg. If a token has already been set up for a TDE, it will add the amount to the previous one. But in case the deposits contains the same addresses the token address will be added again because the last added tokens are not checked. This way some users will receive an excess amount of tokens and the other part will receive less than expected.

Vulnerability Detail

The depositedTokenAddressForTde array is cached before the for loop which reads calldata. The internal for loop checks if the cached array contains an address. If the address is absent it is added to the depositedTokenAddressForTde array. But the cached array is not updated and the cached length is not incremented. So the last added addresses will not be checked on the next cycles of the external for loop.

        address[] memory _tokens = depositedTokenAddressForTde[_actualTDE];
        uint256 tokensLength = _tokens.length;

        for (uint256 i; i < deposits.length; ) {
            IERC20 _token = deposits[i].token;
            uint256 _amount = deposits[i].amount;

            depositedTokenAmountForTde[_actualTDE][_token] += _amount;

            /// @dev Checks whether the token is present in depositedTokenAddressForTde, otherwise we add it.
            bool found;
            for (uint256 j; j < tokensLength; ) {
                if (address(_token) == _tokens[j]) {
                    found = true;
                    break;
                }
                unchecked {
                    ++j;
                }
            }

            if (!found) {
                depositedTokenAddressForTde[_actualTDE].push(address(_token));
            }

The reward amount calculation for a token depends on the share of the user and is determined as a part of the token amount saved at the depositedTokenAmountForTde[_actualTDE][_token] mapping. https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Rewards/YsDistributor.sol#L231-L233 For duplicated addresses users shares will be distributed several times. https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Rewards/YsDistributor.sol#L209-L219

Impact

Incorrect reward distribution.

Code Snippet

https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Rewards/YsDistributor.sol#L108-L130

Tool used

Manual Review

Recommendation

Consider updating the cached array and incrementing the tokensLength variable when a new token is added.

            if (!found) {
                depositedTokenAddressForTde[_actualTDE].push(address(_token));
+               _tokens.push(address(_token));
+               ++tokensLength;
            }

sherlock-audit / 2023-11-convergence-judging

pontifex - Tokens distribution may be broken due to incorrect address verification when depositing tokens #209