Quest owner can withdraw the reward for unclaimed receipt.

[code423n4]

Lines of code

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

Vulnerability details

Impact

Erc1155Quest.withdrawRemainingTokens() will withdraw all tokens even if there are users who minted a receipt but didn't claimed their rewards before endTime

Proof of Concept

• Whitelisted account creates a new ERC1155 quest. Whitelisted account becames the quest owner.
• Owner transfer rewards and execute start function
• Users complete tasks and mints receipts but don't claim them.
• After quest has ended (block.timestamp > endTime), owner calls Erc1155Quest.withdrawRemainingTokens, withdrawing all rewards (rewards who are entitled to receipts owners)
• users can't claim the receipt rewards.

This can happen without a malicious whitelisted account.

Tools Used

Manual review

Recommended Mitigation Steps

Consider the minted receipt number when withdrawing remaining rewards, similar to how it's done for Erc20Quest.

    function withdrawRemainingTokens(address to_) public override onlyOwner {
        super.withdrawRemainingTokens(to_);
        uint256 unclaimedTokens = questFactoryContract.getNumberMinted(questId) - redeemedTokens;
        uint256 amountToWithdraw =  IERC1155(rewardToken).balanceOf(address(this), rewardAmountInWeiOrTokenId) - redeemedTokens;
        IERC1155(rewardToken).safeTransferFrom(
            address(this),
            to_,
            rewardAmountInWeiOrTokenId,
            IERC1155(rewardToken).balanceOf(address(this), rewardAmountInWeiOrTokenId),
            '0x00'
        );
    }

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