ConvexTriCryptoStrategy might not compound all rewards

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/convex/ConvexTricryptoStrategy.sol#L246

Vulnerability details

Impact

When compounding in ConvexTriCryptoStrategy, the number of tokens that is swapped into wETH does not account for extraRewards and tokenRewards. This can cause a loss of yield and rewards to be lost.

Proof of Concept

In ConvexTriCryptoStrategy._executeClaim function restricts the number of tokens to the number of rewardContracts that are claimed from:

require(
    tempData.rewardContracts.length == tempData.tokens.length,
    "ConvexTricryptoStrategy: claim data not valid"
);
...
zap.claimRewards(
    tempData.rewardContracts,
    tempData.extraRewardContracts,
    tempData.tokenRewardContracts,
    tempData.tokenRewardTokens,
    extrasTempData.depositCrvMaxAmount,
    extrasTempData.minAmountOut,
    extrasTempData.depositCvxMaxAmount,
    extrasTempData.spendCvxAmount,
    extrasTempData.options
);

for (uint256 i = 0; i < tempData.tokens.length; i++) {
    finalBalances[i] = balancesAfter[i] - balancesBefore[i];
}

//WARDEN: this data is used in `compound` to swap and compound
return (finalBalances, tempData.tokens);

The call to ClaimZap.claimRewards from Convex will also claim extraRewards and tokenRewards if passed as input and if the strategy is eligible to any of those:

    for(uint256 i = 0; i < extraRewardContracts.length; i++){
        IBasicRewards(extraRewardContracts[i]).getReward(msg.sender);
    }
    //claim from multi reward token contract
    for(uint256 i = 0; i < tokenRewardContracts.length; i++){
        IBasicRewards(tokenRewardContracts[i]).getReward(msg.sender,tokenRewardTokens[i]);
    }

These rewards are not considered in the return data of _executeClaim and hence also not handled in compound.

Tools Used

Manual Review

Recommended Mitigation Steps

Dont restrict the token number to the number of rewardContracts (since most BaseRewardPools pay in the same token CRV, this check does not seem to be correct to begin with). Validate the correctness of the token list differently if necessary.

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as duplicate of #162

[c4-judge]

dmvt marked the issue as satisfactory