code-423n4 / 2023-01-rabbithole-findings

1 stars 2 forks source link

Bad modifier definition #682

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

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 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L98 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L83 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L92-L97

Vulnerability details

Impact

The modifiers holding the name "onlyMinter", defined in the "contracts/RabbitHoleReceipt.sol" and "contracts/RabbitHoleTickets.sol" files do not implement an if or require checks. The lack of checking means that the modifiers do nothing about regulating the use of the "mint" function in the "contracts/RabbitHoleReceipt.sol" file and the "mint", and "mintBatch" functions in the "contracts/RabbitHoleTickets.sol" file respectively. No matter the if the user calling one of the functions has the required privileges or not they will always be allowed to execute the transaction.

Proof of Concept

The definition of the modifier in the "contracts/RabbitHoleReceipt.sol" file: https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L58-L61

The definition of the modifier in the "contracts/RabbitHoleTickets.sol" file: https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L47-L50

Usage of the modifier in the "contracts/RabbitHoleReceipt.sol" file: https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L98

Usage of the modifier in the "contracts/RabbitHoleTickets.sol" file: https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L83

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

Recommended Mitigation Steps

Implement a require or if check. The code in the following examples assumes that the modifiers' functionality is supposed to check whether the user is the minter address. Example with require: require(msg.sender == minterAddress, "Some error message here:"); Example with if: if (msg.sender == minterAddress) revert CustomError();

c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #9

c4-judge commented 1 year ago

kirk-baird marked the issue as satisfactory