Erc20InputFacetStorage Manual Review Findings

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

Type	Severity	Location
Input Sanitization		Erc20InputFacetStorage.sol:L148-L166

Description:

The Erc20InputFacetStorage::setCampaignTransferInput 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 token multiple times will solely retain the last amount per packet configured which is incorrect.

Example:

Copy
148for (uint256 i = 0; i < length; i++) {
149    address tokenAddress = tokenAddresses_[i];
150    uint256 amountPerPacket = amountOfTokensPerPacket_[i];
151
152    if (tokenAddress == address(0)) {
153        revert InvalidZeroInputPacketAddressData(campaignId, i);
154    }
155    if (amountPerPacket == 0) {
156        revert InvalidZeroInputPacketAmountData(campaignId, i);
157    }
158
159    // Call InputAssetKeeper for maximum approval
160    bytes memory data = abi.encodeWithSignature("approve(address,uint256)", address(this), type(uint256).max);
161
162    ICampaignAssetManager(campaignInfo.inputAssetKeeper).execute(tokenAddress, data);
163
164    campaignPacketInfo.tokenAddresses[i] = tokenAddress;
165    campaignPacketInfo.amountOfTokensPerPacket[tokenAddress] = amountPerPacket;
166}

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.