RabbitHole Quest Protocol contest - millersplanet'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

Unrestricted minting of RabbitHoleReceipt tokens (ERC-721) due to improper access control

The RabbitHoleReceipt.sol contract has a onlyMinter() modifier attempting to allow a specific address only to be allowed to mint. However, the implementation for the modifier is not correct and allows any address to bypass it because it does not revert in case the minterAddress != msg.sender:

modifier onlyMinter() {
	msg.sender == minterAddress; 
	_;
}

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

Impact

Improper access control on incorrectly implemented modifier allows any caller to mint tokens on the protocol at any given time, ultimately rendering the value of the NFTs to zero.

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

Proof of Concept

• Bob the attacker (or any curios user) calls the mint() function passing his address in the to_ parameter assigning any uncreated questId_ he likes.
• He does it again. And again.

Tools Used

• Manual review

Recommended Mitigation Steps

Place an if() statement to check that the msg.sender is minterAddress and revert if it isn't.

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

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

Vulnerability details

Unrestricted minting of RabbitHoleTickets tokens (ERC-1155) due to improper access control

The RabbitHoleTickets.sol contract has a onlyMinter() modifier attempting to allow a specific address only to be allowed to mint. However, the implementation for the modifier is not correct and allows any address to bypass it because it does not revert in case the minterAddress != msg.sender:

modifier onlyMinter() {
	msg.sender == minterAddress; 
	_;
}

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

Impact

Improper access control on incorrectly implemented modifier allows any caller to mint tokens on the protocol at any given time, ultimately rendering the value of the NFTs to zero.

Vulnerable mint() function: https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L83-L85

Vulnerable mintBatch()) function: https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleTickets.sol#L92-L99

Proof of Concept

• Bob the attacker (or any curios user) calls the mintBatch() function passing his address in the to_ parameter and sending any _amounts of token he likes, passing any arbitrary _data that he likes as well.
• He does it again. And again.

Tools Used

• Manual review

Recommended Mitigation Steps

Place an if() statement to check that the msg.sender is minterAddress and revert if it isn't.

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