Closed code423n4 closed 1 year ago
https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L48 https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L47-L50 https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L92 https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L83 https://github.com/rabbitholegg/quest-protocol/blob/bd213e8629bb8587dd4bb35f3e9e8f8e42b40336/contracts/RabbitHoleReceipt.sol#L98
Modifier is wrongly implemented, so every function that uses onlyMinter will be callable by anyone.
onlyMinter
This affects:
RabbitHoleTickets#mintBatch()
RabbitHoleTickets#mint()
RabbitHoleReceipt#mint()
Modifier has no if + revert / require statement within the condition, therefore, it will always pass the "checks"
if
revert
require
modifier onlyMinter() { msg.sender == minterAddress; _; }
This means one can mint all the tokens / NFTs he wants and claim rewards after that (Claim flow).
Minting NFTs even if it shouldn't be able. Callable as much as wanted, direct impact on the value of the protocol.
https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L48
https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L47-L50
Manual Review
modifier onlyMinter() { - msg.sender == minterAddress; + if(msg.sender != minterAddress) revert NOT_MINTER_ERROR; _; }
kirk-baird marked the issue as duplicate of #9
kirk-baird marked the issue as satisfactory
Lines of code
https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L48 https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L47-L50 https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L92 https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L83 https://github.com/rabbitholegg/quest-protocol/blob/bd213e8629bb8587dd4bb35f3e9e8f8e42b40336/contracts/RabbitHoleReceipt.sol#L98
Vulnerability details
Minting can be called by anyone
Summary
Modifier is wrongly implemented, so every function that uses
onlyMinter
will be callable by anyone.This affects:
RabbitHoleTickets#mintBatch()
RabbitHoleTickets#mint()
RabbitHoleReceipt#mint()
Vulnerability Detail
Modifier has no
if
+revert
/require
statement within the condition, therefore, it will always pass the "checks"This means one can mint all the tokens / NFTs he wants and claim rewards after that (Claim flow).
Impact
Minting NFTs even if it shouldn't be able. Callable as much as wanted, direct impact on the value of the protocol.
Code Snippet
https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L48
https://github.com/rabbitholegg/quest-protocol/blob/32a9b58f040442af4c7f459fe409e40af0e54a78/contracts/RabbitHoleTickets.sol#L47-L50
Tool used
Manual Review
Recommendation