The old removed `rewardToken` may be left in the contract

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-xeth/commit/1f714868f193cdeb472ec097110901a997d87ec4#L1

Vulnerability details

The lack of a mechanism to modify rewardTokens[] If convex adds new extraRewards CVXStaker.sol cannot transfer the added token

Mitigation

https://github.com/code-423n4/2023-05-xeth/commit/1f714868f193cdeb472ec097110901a997d87ec4

This PR has added the method setRewardTokens to modify rewardTokens[].

But the current implementation has a problem, it doesn't transfer the old rewardTokens[] first so that if rewardTokens[] is changed from more to less. (convex is possible to become less) Since the transfer of the old rewardTokens[] is not triggered, the token to be removed may have already been generated and stored in the contract When new tokens are set, transferReward() does not transfer the old token.(Although the owner can take it out by recoverToken, maybe the owner doesn't even notice it) So it is recommended to trigger the transfer of the old rewardTokens[] first

function setRewardTokens(address[] calldata newTokens) external onlyOwner {
+   getReward(true);
+   transferReward(0,rewardTokens.length);

    rewardTokens = newTokens;

    emit SetRewardTokens(newTokens);
}

Assessed type

Context

[c4-judge]

kirk-baird marked the issue as unmitigated

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird marked the issue as new finding

[c4-judge]

kirk-baird marked the issue as confirmed for report