ERC20 quest does not support rebasing tokens

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/Erc20Quest.sol#L66-L68
https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/Erc20Quest.sol#L81-L87
https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/Erc20Quest.sol#L102-L104

Vulnerability details

Impact

When a rebasing token is used as the reward token for an ERC20 quest, it is possible that this token's rebasing event, which reduces its balance owned by the Erc20Quest contract, occurs before the following Erc20Quest._transferRewards, Erc20Quest.withdrawRemainingTokens, and Erc20Quest.withdrawFee functions are called. When this occurs, calling these transfer functions can revert due to insufficient balance of this rebasing reward token owned by the Erc20Quest contract. As a result, the reward token amounts that suppose to be received by the RabbitHole receipt holders, quest owner, and/or protocol fee recipient cannot be transferred.

https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/Erc20Quest.sol#L66-L68

    function _transferRewards(uint256 amount_) internal override {
        IERC20(rewardToken).safeTransfer(msg.sender, amount_);
    }

https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/Erc20Quest.sol#L81-L87

    function withdrawRemainingTokens(address to_) public override onlyOwner {
        super.withdrawRemainingTokens(to_);

        uint unclaimedTokens = (receiptRedeemers() - redeemedTokens) * rewardAmountInWeiOrTokenId;
        uint256 nonClaimableTokens = IERC20(rewardToken).balanceOf(address(this)) - protocolFee() - unclaimedTokens;
        IERC20(rewardToken).safeTransfer(to_, nonClaimableTokens);
    }

https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/Erc20Quest.sol#L102-L104

    function withdrawFee() public onlyAdminWithdrawAfterEnd {
        IERC20(rewardToken).safeTransfer(protocolFeeRecipient, protocolFee());
    }

Proof of Concept

The following steps can occur for one of the described scenarios.

1. An ERC20 quest is created for one total participant and zero protocol fee with a rebasing token as its reward token.
2. Alice is provided with the signature for minting one RabbitHole receipt for this quest. Then, she mints one receipt accordingly.
3. Before Alice claims the rewards associated with the minted receipt, the reward token's rebasing event occurs that reduces its balance owned by the Erc20Quest contract.
4. Alice calls the claim function for this quest but this function call reverts because the rebasing reward token's balance owned by the Erc20Quest contract is now insufficient to cover her reward token amount. Hence, she is unable to receive the reward token amount associated with her RabbitHole receipt.

Tools Used

VSCode

Recommended Mitigation Steps

Like some other protocols, this protocol does not have to support rebasing tokens. A blocklist can be used to block the usage of these tokens. If blocking these tokens, please explicitly document it so users know about this.

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

This previously downgraded issue has been upgraded by kirk-baird

[c4-judge]

kirk-baird marked the issue as duplicate of #630

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird marked the issue as selected for report

[c4-sponsor]

waynehoover marked the issue as sponsor disputed

[waynehoover]

We have a rewardAllowlist in which we will not include rebasing tokens, our protocol does not support rebasing tokens.

[kirk-baird]

Reward allow list is a genuine solution to prevent rebasing and FoT tokens and so I'm going to downgrade this to a QA.

I'd also recommend clearly documenting the rebasing and FoT tokens will not be allowed.

[c4-judge]

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

[c4-judge]

kirk-baird marked the issue as grade-a

[c4-judge]

kirk-baird marked the issue as not selected for report