Wrongly implemented modifier allow everybody to mint Rabbit Hole tickets.

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L47-L50
https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L83
https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L97

Vulnerability details

Impact

As specified on RabbitHole C4 contest page, RabbitHoleTickets smart contract 'is an 1155 reward contract used by the RabbitHole team.' Meaning that the assets managed by this smart contract have value. Moreover this contract implements ERC-2981: NFT Royalty Standard meaning that the tickets can be traded for other assets.
RabbitHoleTickets.onlyMinter doesn't check if msg.sender is the minter address.
Anybody can call RabbitHoleTickets.mint (or RabbitHoleTickets.mintBatch) to mint unlimited number of tokens. Being a token with infinite supply it will lose value and it's creator can lose reputation.

Proof of Concept

• Attacker calls mintBatch / mint and mint a huge number of tickets.
• Dump minted tickets on secondary markets for profit.

Tools Used

Manual review

Recommended Mitigation Steps

The fix is simple, properly implement the modifier as:

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

[c4-judge]

kirk-baird marked the issue as duplicate of #9

[c4-judge]

kirk-baird marked the issue as satisfactory