code423n4 commented 1 year ago

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 commented 1 year ago

kirk-baird marked the issue as duplicate of #9