RabbitHole Quest Protocol contest - bytes032's results

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L96 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L102 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L76

Vulnerability details

Lines of Code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L96 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L102 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L76

Vulnerability details

The function withdrawFee(...) is called once a quest has ended (the final step in the diagram)

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

modifier onlyAdminWithdrawAfterEnd() {
 if (block.timestamp < endTime) revert NoWithdrawDuringClaim();
 _;
}

I'm assuming, given the name, the modifier is supposed to be called only by the administrators after the quest has ended. However, the latter is not true. The function can be called by anyone, given the quest has ended.

Additionally, the function can be called indefinitely to drain the number of rewardTokens in ERC20Quest, because it uses the protocolFee() which is a view function that always produces the same value as an amount to be transferred.

It's important to note 2 things:

1. protocolFeeRecipient is set and owned by the RabbitHole protocol by a function which has onlyOwner modifier
2. I'm assuming the reason for calculating the protocolFee() with a view function is that after the quest ends there people who still have unclaimed tokens/rewards.

The following vulnerability leads the following scenario:

Quest is ended, but a malicious user keeps calling withdrawFee(), essentially preventing anybody to claim their funds. Even if the protocol keeps funding back the quest, the user can keep spamming the withdrawFee function. Depending on the user's goal, he can drain protocolFeeRecipient any funds in the contract.

As a result, regular users will have difficulty claiming their tokens and getting their rewards.

Quest ends, but RH keeps spamming the withdrawFee function, essentially rugging the quest and stealing all the money.

POC

Scenario 1

• Quest ends
• Malicious user keeps spamming withdrawFee(), rendering the withdrawal mechanism useless.

Scenario 2:

• Quest ends
• RH keeps spamming withdrawFee until the quest is empty, rugging the creator of the quest and stealing all the tokens.

Tools

Manual testing

Recommended Mitigation Steps

First, ensure that the modifier is only callable by admins. Second, introduce a maximum period upon which users can claim tokens. After the period ends, forbid claiming. Once you forbid claiming, compute the result that is supposed to come from protocolFee().

Then, when withdrawFee() is called, check if the period has ended, set the result computed in the previous step to 0 and transfer the fee as intended.