Missing custom error or require statement in onlyMinter modifier

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L48
https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L59

Vulnerability details

Impact

Without custom error or require statement at modifier onlyMinter, anyone could mint a receipt in the RabbitHoleReceipt contract or a ticket in the RabbitHoleTickets contract.

Proof of Concept

For RabbitHoleReceipt add a test in RabbitHoleReceipt.spec.ts

describe('additional mint test', () => {
        it('check onlyMinter modifier', async () => {
              await RHReceipt.connect(minterAddress).mint(firstAddress.address, 'def456')
              expect(await RHReceipt.balanceOf(firstAddress.address)).to.eq(1)

              await RHReceipt.connect(firstAddress).mint(firstAddress.address, 'def456')
              expect(await RHReceipt.balanceOf(firstAddress.address)).to.eq(2)

              await RHReceipt.connect(royaltyRecipient).mint(firstAddress.address, 'def456')
              expect(await RHReceipt.balanceOf(firstAddress.address)).to.eq(3)

              expect(minterAddress.address != firstAddress.address && minterAddress.address != royaltyRecipient.address).to.equal(true)
         })
})

For RabbitHoleTickets add a test in RabbitHoleTickets.spec.ts

describe('additional mint test', () => {
        it('check onlyMinter modifier', async () => {
              await RHTickets.connect(minterAddress).mint(firstAddress.address, 1, 5, "0x")
              expect(await RHTickets.balanceOf(firstAddress.address, 1)).to.eq(5)

              await RHTickets.connect(firstAddress).mint(firstAddress.address, 1, 5, "0x")
              expect(await RHTickets.balanceOf(firstAddress.address, 1)).to.eq(10)

              await RHTickets.connect(royaltyRecipient).mint(firstAddress.address, 1, 5, "0x")
              expect(await RHTickets.balanceOf(firstAddress.address, 1)).to.eq(15)

              expect(minterAddress.address != firstAddress.address && minterAddress.address != royaltyRecipient.address).to.equal(true)
        })
})

As it is seen from these tests code, it is possible from anyone to call the mint operation in the RabbitHoleReceipt and RabbitHoleTickets (also, there is an additional except expect(minterAddress.address != firstAddress.address && minterAddress.address != royaltyRecipient.address).to.equal(true) which shows that addresses that call mint operation are different than minter address).

Tools Used

VSCode, Hardhat, Solidity Visual Developer plugin for VSCode

Recommended Mitigation Steps

Create and add a custom error in the if statement for a check is msg.sender different than minterAddress at lines RabbitHoleTickets.sol#L48 and RabbitHoleReceipt.sol#L59

modifier onlyMinter() {
       if (msg.sender != minterAddress) revert OnlyMinterAllowed();
       _;
}

[c4-judge]

kirk-baird marked the issue as duplicate of #9

[c4-judge]

kirk-baird marked the issue as satisfactory