Description

We note that this particular finding addresses an issue with a contract that is outside of the scope of the audit.

The mint function of an ERC-1155 contract performs a so-called acceptance check on contracts that are receiving tokens.

For this check to be properly implemented, the contract that is receiving tokens must implement the IERC1155Receiver interface. This interface includes the function onERC1155Received, which is called by the mint function of the ERC-1155 contract. This procedure is potentially problematic, as malicious actors can implement custom onERC1155Received functions that reenter the calling sequence of the ERC-1155 contract.

In this case, the _claim function performs a mint call on the rewardNFT contract. This call can be reentered by a malicious actor, which can lead to a reentrancy attack, potentially minting multiple tokens of the same tokenId to the same user.

function _claim(uint256 _campaignId, address _user) internal returns (uint128) {
    require(isActiveCampaign[_campaignId], "RewardCampaignManager: !_campaignId");
    uint128 count = 0;
    uint256[] memory tokenIds = rewardNFT.getAllTokenIds();
    uint256 len = tokenIds.length;
    for (uint256 i = 0; i < len; ++i) {
        if (users[_campaignId][tokenIds[i]].contains(_user) && !isClaimedUser[_campaignId][tokenIds[i]][_user]) {
            rewardNFT.mint(_user, tokenIds[i], 1);
            isClaimedUser[_campaignId][tokenIds[i]][_user] = true;
            count++;
        }
    }
    return count;
}

Impact

A malicious user can exploit this vulnerability to mint multiple tokens of the same tokenId, potentially leading to further issues involving the claimed tokens.

Recommendations

We recommend moving the isClaimedUser status update before the mint call to prevent reentrancy attacks.

function _claim(uint256 _campaignId, address _user) internal returns (uint128) {
    require(isActiveCampaign[_campaignId], "RewardCampaignManager: !_campaignId");
    uint128 count = 0;
    uint256[] memory tokenIds = rewardNFT.getAllTokenIds();
    uint256 len = tokenIds.length;
    for (uint256 i = 0; i < len; ++i) {
        if (users[_campaignId][tokenIds[i]].contains(_user) && !isClaimedUser[_campaignId][tokenIds[i]][_user]) {
-           rewardNFT.mint(_user, tokenIds[i], 1);
-           isClaimedUser[_campaignId][tokenIds[i]][_user] = true;
+           isClaimedUser[_campaignId][tokenIds[i]][_user] = true;
+           rewardNFT.mint(_user, tokenIds[i], 1);
            count++;
        }
    }
    return count;
}

This way, the isClaimedUser status is updated before the mint call, preventing reentrancy attacks.

Remediation

This issue has been acknowledged by WOOFI, and a fix was implemented in commit 7d453754↗.