RabbitHole Quest Protocol contest - p4st13r4's results

Lines of code

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

Vulnerability details

Impact

Affected code:

• https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L96
• https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L99

The claim() function of a Quest calls into the RabbitHoleReceipt contract to check which receipts belong to the msg.sender for the given questId, and then proceeds to pay the unclaimed rewards to the user.

An issue may arise for users that participated in many quests and thus have a lot of past receipts, because the getOwnedTokenIdsOfQuest() function used in RabbitHoleReceipt iterates through all the user’ past receipts, and then filters for the given questId. If the list of receipts gets too big, then an user won’t be able to be rewarded for all the present and future quests, since the claim() function will run out of gas

Proof of Concept

This is a test added in test/Erc20Quest.spec.ts, within the claim() group of tests:

it.only('should run out of gas if you partecipated in too many quests', async () => {
  // Mint 6k "past" receipts
  const receiptsPromises = []
  for (let i = 0; i < 6000; i++) {
    let randomQuestId = (Math.random() + 1).toString(36).substring(2)
    receiptsPromises.push(deployedRabbitholeReceiptContract.mint(owner.address, randomQuestId))
  }

  await Promise.all(receiptsPromises)

  // Mint one additional receipt for current quest
  await deployedRabbitholeReceiptContract.mint(owner.address, questId)
  await deployedQuestContract.start()

  await ethers.provider.send('evm_increaseTime', [86400])

  expect(await deployedSampleErc20Contract.balanceOf(owner.address)).to.equal(0)
  expect(await deployedQuestContract.isClaimed(1)).to.equal(false)

  // @audit: Will run out of gas
  await deployedQuestContract.claim()
  expect(await deployedSampleErc20Contract.balanceOf(owner.address)).to.equal(rewardAmount)
  await ethers.provider.send('evm_increaseTime', [-86400])
})

The test fails because claim() exceeds 30M gas usage

Tools Used

Editor

Recommended Mitigation Steps

Build a separate data structure in RabbitHoleReceipt to help deal with already redeemed receipts