RabbitHole Quest Protocol contest - yosuke's results

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/RabbitHoleReceipt.sol#L98-L104

Vulnerability details

Handle

yosuke

Impact

The mint function in RabbitHoleReceipt.sol uses the onlyMinter modifier. This modifier is shown below, and the revert is not working, so anyone can pass the check.

modifier onlyMinter() {
    msg.sender == minterAddress; //Where is revert()?
    _;
}

Furthermore, since the mint function is public and can be easily called from outside the contract, an attacker who is not on-chain-tasked can mint multiple receipts. An attacker who mints them can exchange them for ERC20 or ERC1155, draining funds from the contract and fatally damaging the protocol. Also, those who are originally entitled to the rewards will not get them. For these reasons, I consider this bug to be highly serious.

Proof of Concept

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L58-L61 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L98-L104

Tools Used

pen and paper

Recommended Mitigation Steps

Please fix the onlyMinter modifier as shown below.

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