Erc1155Quest's withdrawRemainingTokens doesn't take in consideration unclaimed tokens

[code423n4]

Lines of code

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

Vulnerability details

Impact

Erc1155Quest's withdrawRemainingTokens function allows the owner to claim all the remaining tokens, the unclaimed tickets will become worthless since they won't be claimable. Note this functionality for the Erc20Quest correctly take in consideration the unclaimed ticket remaining.

Proof of Concept

The quest ended and there are 100 unclaimed tickets corresponding to 100 ERC1155 token rewards. The owner calls withdrawRemainingTokens and withdraw all the remaining tokens, the unclaimed tickets can't be claim anymore.

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

Recommended Mitigation Steps

In withdrawRemainingTokens the correct amount of tokens to withdraw should be contract's ERC1155 tokens balance - unclaimed tokens (questFactoryContract.getNumberMinted(questId) - redeemedTokens).

[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)