Indexes in recoverTokens() might not match TokenAmounts()
Description
The recoverTokens() function is intended to be used by the owner to recover tokens from the contract. This might be required if the tokens have been in the contract for a long time without being claimed. The owner passes in a list of indexes that will be marked as "claimed" and a list of tokens and their corresponding amounts to be recovered.
The current implementation of recoverTokens() lacks a check to ensure that the indexes provided by the owner correspond to the token amounts being recovered.
function recoverTokens(uint[] calldata indexList, TokenAmount[] calldata tokenAmounts) external onlyOwner nonReentrant {
for (uint i = 0; i < indexList.length; ++i) {
uint index = indexList[i];
require(!alreadyClaimed[index], "already claimed");
alreadyClaimed[index] = true;
emit ClaimedByOwner(index);
}
for (uint i = 0; i < tokenAmounts.length; ++i) {
SafeERC20.safeTransfer(IERC20(tokenAmounts[i].token), owner, tokenAmounts[i].amount);
}
}Impact
The state tracking of the token balances in the contract may be left in an inconsistent state.
Recommendations
Merkle proofs should be used to ensure that the indexes passed in match the token amounts being recovered. The number of items in both the indexList and the recoverTokensList should be checked to be the same.
An example implementation is shown below:
struct RecoverTokens {
uint index;
address from;
TokenAmount[] tokenAmounts;
bytes32[] proof;
}
function recoverTokens(RecoverTokens[] calldata recoverTokensList) external onlyOwner nonReentrant {
for (uint i = 0; i < recoverTokensList.length; ++i) {
uint index = recoverTokensList[i].index;
bytes32[] calldata proof = recoverTokensList[i].proof;
address from = recoverTokensList[i].from;
TokenAmount[] calldata tokenAmounts = recoverTokensList[i].tokenAmounts;
require(!alreadyClaimed[index], "already claimed");
alreadyClaimed[index] = true;
emit ClaimedByOwner(index);
require(MerkleProof.verify(proof, merkleRoot, keccak256(abi.encode(index, from, tokenAmounts))), "proof invalid");
for (uint i = 0; i < tokenAmounts.length; ++i) {
SafeERC20.safeTransfer(IERC20(tokenAmounts[i].token), owner, tokenAmounts[i].amount);
}
}
}Remediation
This issue has been acknowledged by Euler Labs Ltd..