Erc20RewardTransferFacetStorage Manual Review Findings

ERR-01M: Inexistent Prevention of Token Re-Configuration

Type	Severity	Location
Input Sanitization		Erc20RewardTransferFacetStorage.sol:L143

Description:

The Erc20RewardTransferFacetStorage::setCampaignTransferRewards function does not prevent the same token from being configured twice, causing it to be duplicated in the tokenAddresses array but retain its last amountPerPacket configuration defined.

Impact:

A campaign configured with the same reward token multiple times will solely retain the last amount per packet configured which is incorrect.

Example:

Copy
126for (uint256 i = 0; i < length; i++) {
127    address tokenAddress = tokenAddresses_[i];
128    uint256 amountPerPacket = amountOfTokensPerPacket_[i];
129
130    if (tokenAddress == address(0)) {
131        revert InvalidZeroRewardPacketAddressData(campaignId, i);
132    }
133    if (amountPerPacket == 0) {
134        revert InvalidZeroRewardPacketAmountData(campaignId, i);
135    }
136
137    // Call Reward Asset Handler for maximum approval
138    bytes memory data = abi.encodeWithSignature("approve(address,uint256)", address(this), type(uint256).max);
139
140    ICampaignAssetManager(campaignInfo.rewardAssetHandler).execute(tokenAddress, data);
141
142    campaignPacketInfo.tokenAddresses[i] = tokenAddress;
143    campaignPacketInfo.amountOfTokensPerPacket[tokenAddress] = amountPerPacket;
144}

Recommendation:

We advise such a case to be prohibited, as the same token being configured once for 20 units and once for 30 units should ultimately require 50 units instead of 60.

Alternatively, we advise such a case to be handled properly by incrementing the amountOfTokensPerPacket by the relevant amountPerPacket required.

Alleviation (b64b659786cf3c84bea52feb3a69f546ba3601f0):

A token's reconfiguration is now prevented by keeping track of an isTokenAddressImported mapping, preventing the scenario outlined in the exhibit.