code-423n4 / 2023-01-rabbithole-findings

1 stars 2 forks source link

Upgraded Q -> 3 from #621 [1675724753994] #696

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #621 as 3 risk. The relevant finding follows:

L1 - Owner could withdraw all unclaimed tokens while some still should be claimable withdrawRemainingTokens() function in the Erc1155Quest contract allows the owner to withdraw all remaining tokens, including unclaimed ones that may still be claimable in the future. This could result in the accidental withdrawal of tokens that are meant to remain on the contract balance until claimed by users.

File: Erc1155Quest.sol 52: /// @dev Withdraws the remaining tokens from the contract. Only able to be called by owner 53: /// @param to The address to send the remaining tokens to 54: function withdrawRemainingTokens(address to) public override onlyOwner { 55: super.withdrawRemainingTokens(to); 56: IERC1155(rewardToken).safeTransferFrom( 57: address(this), 58: to, 59: rewardAmountInWeiOrTokenId, 60: IERC1155(rewardToken).balanceOf(address(this), rewardAmountInWeiOrTokenId), 61: '0x00' 62: ); 63: } Recommended Mitigation Steps

Consider adding tracking flow in Erc1155Quest contract withdrawRemainingTokens function similar to Erc20Quest withdrawing function:

function withdrawRemainingTokens(address to_) public override onlyOwner { 
    super.withdrawRemainingTokens(to_);
    uint256 remainingTokens = totalParticipants - questFactoryContract.getNumberMinted(questId);
    IERC1155(rewardToken).safeTransferFrom(
        address(this),
        to_,
        rewardAmountInWeiOrTokenId,
        remainingTokens,
        '0x00'
    );
}
c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #42

c4-judge commented 1 year ago

This auto-generated issue was withdrawn by kirk-baird

c4-judge commented 1 year ago

This previously downgraded issue has been upgraded by kirk-baird

c4-judge commented 1 year ago

kirk-baird marked the issue as not a duplicate

c4-judge commented 1 year ago

kirk-baird marked the issue as unsatisfactory: Invalid