Winners of ERC1155Quest might not be able to claim rewards after the quest end (losing fund)

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc1155Quest.sol#L54-L63

Vulnerability details

Impact

Detailed description of the impact of this finding.
Winners of ERC1155Quest might not be able to claim rewards after the quest end if the host calls the function withdrawRemainingTokens() right after the quest end.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Winners of ERC1155Quest might not be able to claim rewards after the quest end if the host calls the function withdrawRemainingTokens() right after the quest end. This is because withdrawRemainingTokens() will withdraw all remaining tokens in the contract, including those that have not yet been claimed:

function withdrawRemainingTokens(address to_) public override onlyOwner {
        super.withdrawRemainingTokens(to_);
        IERC1155(rewardToken).safeTransferFrom(
            address(this),
            to_,
            rewardAmountInWeiOrTokenId,
            IERC1155(rewardToken).balanceOf(address(this), rewardAmountInWeiOrTokenId),
            '0x00'
        );
    }

withdrawRemainingTokens() should have left the unclaimed tokens in the contract so that winners can claim them. Otherwise they lost fund.

Tools Used

Remix

Recommended Mitigation Steps

Similar to its counterpart for ERC20Quest, leave the unclaimed tokens in the contract for the winners to claim. If necessary, implement another function called withdrawUnclaimedTokens() that will be called after some claimDeadline expires.

[c4-judge]

kirk-baird marked the issue as duplicate of #42

[c4-judge]

kirk-baird changed the severity to QA (Quality Assurance)

[c4-judge]

This previously downgraded issue has been upgraded by kirk-baird

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird changed the severity to 2 (Med Risk)