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

1 stars 2 forks source link

Minter address vulnerability `RabbitHoleReceipt.sol` #18

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

Vulnerability details

Impact

modifier onlyMinter() {
    msg.sender == minterAddress;
    _;
}
function mint(address to_, string questId_) public onlyMinter {
    //...
}
function setMinterAddress(address minterAddress_) public onlyOwner {
    minterAddress = minterAddress_;
    emit MinterAddressSet(minterAddress_);
}

This modifier is used to restrict the mint() function to be called only by the minter address, which is set during the contract initialization, but also any other functions that use this modifier. It also allows the minter address to be changed by the contract owner using the setMinterAddress() function.

If the minter address is compromised or controlled by a malicious actor, it could lead to the creation of unauthorized tokens, which could be used to perform fraudulent activities, such as double spending or counterfeiting.

Proof of Concept

An attacker could exploit this vulnerability by first compromising or obtaining control over the minter address. Once in control of the minter address, the attacker could then call the mint() function to mint new tokens, potentially creating a large number of tokens. These tokens could then be used to perform fraudulent activities, such as double spending or counterfeiting.

The attacker could also call the setMinterAddress() function to change the minter address to a malicious address, allowing the attacker to continue to perform unwanted actions on the contract, such as minting new tokens or changing the minter address again.

Code Snippet

// Assume the attacker has compromised the minter address
address compromisedMinterAddress = msg.sender;

// Change the minter address to the attacker's address
RabbitHoleReceipt contract = RabbitHoleReceipt(contractAddress);
contract.setMinterAddress(compromisedMinterAddress);

// Mint a large number of tokens using the compromised minter address
for (uint i = 0; i < 10; i++) {
    contract.mint(attacker, "questId");
}

The attacker first obtains control of the minter address and then changes the minter address to their own address. They then use the compromised minter address to mint a large number of tokens, potentially causing issues on the contract.

In some scenarios, the attacker could also prevent the legitimate minter to mint new tokens by keeping the minter address under his control.

Tools Used

Manual audit, Vs Code.

Recommended Mitigation Steps

First possible option is to use a multisig contract as the minter address. This would require multiple parties to agree on any changes to the minter address, making it more difficult for a single party to compromise the minter address.

// Use a multisig contract as the minter address
address multisig = MultisigWallet(multisigAddress);
contract.setMinterAddress(multisig);

Another option is to limit the number of tokens that can be minted by the minter address. This can be done by adding a maximum token supply variable and checking it before minting new tokens.

// Limit the number of tokens that can be minted
uint256 public maxSupply = 100;

function mint(address to, string questId) public onlyMinter {
    require(totalSupply <= maxSupply, "Maximum token supply reached");
    // Mint new token
    _mint(to, _tokenIds.next());
    questIdForTokenId[_tokenIds.current()] = questId;
    timestampForTokenId[_tokenIds.current()] = now;
}

Another way is to use a time-lock mechanism that prevents the minter address from changing for a certain period of time, which would make it more difficult for an attacker to compromise the minter address and perform unwanted actions.


// Time-lock mechanism that prevents the minter address from changing for a certain period of time
uint256 public minterChangeTimeout = 30 days;

function setMinterAddress(address newMinterAddress) public onlyOwner {
    require(now >= lastMinterChangeTimestamp + minterChangeTimeout, "Timeout not reached");
    minterAddress = newMinterAddress;
    lastMinterChangeTimestamp = now;
}
c4-judge commented 1 year ago

kirk-baird marked the issue as unsatisfactory: Invalid