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

1 stars 2 forks source link

Upgraded Q -> 3 from #648 [1675725284542] #697

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #648 as 3 risk. The relevant finding follows:

  1. Incorrect Minter Address Validation in Mint Function Link : https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L98

Summary:

The mint function in the RabbitHoleReceipt contract does not correctly check the msg.sender address for minter permissions. The onlyMinter modifier is used to check if the msg.sender is equal to the contract's minter address, but the msg.sender variable is never updated within the modifier. As a result, the check will always return false and any address will be able to mint new tokens.

Impact:

This vulnerability allows any address to mint new tokens without proper authorization. This could result in an attacker creating an arbitrary number of tokens and potentially reselling them. Additionally, the questIdForTokenId and timestampForTokenId mappings will be updated with invalid values, which may cause issues with later functions that rely on this data.

Recommendation:

The onlyMinter modifier should be updated to use the require statement to check if the msg.sender address is equal to the minter address, rather than just a comparison. A sample fix would be:

modifier onlyMinter() { require(msg.sender == minterAddress, "Sender is not authorized minter"); _; } Example:

modifier onlyMinter() { require(msg.sender == minterAddress, "Sender is not authorized minter"); _; }

function mint(address to, string memory questId) public onlyMinter { _tokenIds.increment(); uint newTokenID = tokenIds.current(); questIdForTokenId[newTokenID] = questId; timestampForTokenId[newTokenID] = block.timestamp; safeMint(to, newTokenID); }

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