`ArcadeToken::setMinter` and some functions from `ArcadeTokenDistributor` should use a two-step process

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-arcade/blob/f8ac4e7c4fdea559b73d9dd5606f618d4e6c73cd/contracts/token/ArcadeToken.sol#L132-L137 https://github.com/code-423n4/2023-07-arcade/blob/f8ac4e7c4fdea559b73d9dd5606f618d4e6c73cd/contracts/token/ArcadeTokenDistributor.sol#L73-L82 https://github.com/code-423n4/2023-07-arcade/blob/f8ac4e7c4fdea559b73d9dd5606f618d4e6c73cd/contracts/token/ArcadeTokenDistributor.sol#L89-L98 https://github.com/code-423n4/2023-07-arcade/blob/f8ac4e7c4fdea559b73d9dd5606f618d4e6c73cd/contracts/token/ArcadeTokenDistributor.sol#L105-L114 https://github.com/code-423n4/2023-07-arcade/blob/f8ac4e7c4fdea559b73d9dd5606f618d4e6c73cd/contracts/token/ArcadeTokenDistributor.sol#L121-L130 https://github.com/code-423n4/2023-07-arcade/blob/f8ac4e7c4fdea559b73d9dd5606f618d4e6c73cd/contracts/token/ArcadeTokenDistributor.sol#L138-L147 https://github.com/code-423n4/2023-07-arcade/blob/f8ac4e7c4fdea559b73d9dd5606f618d4e6c73cd/contracts/token/ArcadeTokenDistributor.sol#L155-L164

Vulnerability details

ArcadeToken::setMinter should be a two-step process - any mistake when setting the minter may lead to minter set to an address with unknown private key and it won't be possible to mint new tokens. Recovery will also be impossible since new minter can be only set by the current minter.

The two-step process would look like as follows:

• setMinter is called with some address X and a variable called pendingMinter, for example, will be set to X
• in order to become a new minter, X would have to call a function called becomeMinter (or some other, similar name), which would check if X == msg.sender and will change minter if that condition is satisfied

Such a process is easy to implement and will prevent situations as the one described above, in the first paragraph.

Similar process should be implemented for the following functions from ArcadeTokenDistributor:

• toTreasury
• toDevPartner
• toCommunityRewards
• toCommunityAirdrop
• toTeamVesting
• toPartnerVesting

If wrong address is supplied in any of these functions, entire ArcadeToken balance that should be distributed to some entity / group of people will be irrecoverably lost. Target address which is entitled to receive ArcadeTokens should be able instead to call a function ArcadeTokenDistributor::claim (or a function with a similar name), which would just transfer tokens to msg.sender if it is correct.

Impact

ArcadeToken::minter set to an address with unknown private key will result in inability to mint new ArcadeTokens - it clearly affects protocol functionality.

Any mistake in the functions from ArcadeTokenDistributor enumerated above will result in a loss of funds.

Since protocol functionality may be affected and money can be lost (hence the impact is severe) and the issue requires an external factor (wrong address has to be provided), I'm submitting it as Medium.

Proof of Concept

Consider the following scenarios:

1. Current ArcadeToken::minter decides to set the new minter, but the wrong address is specified.
2. It won't be possible to mint new ArcadeTokens anymore.

Or, the second one:

1. toTreasury function is called with a wrong address.
2. ArcadeToken allocation for the treasury (25.5% of initial distribution) will be irrecoverably lost.

Tools Used

VS Code

Recommended Mitigation Steps

Implement two-step process in order to make sure that even when a wrong address is specified, it won't negatively affect the protocol.

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as primary issue

[141345]

can set back.

QA might be more appropriate.

[c4-sponsor]

PowVT marked the issue as sponsor acknowledged

[PowVT]

setMinter and mint tokens will have very high custom thresholds when governance is released. The distributor contract will be owned by a multisig and require multiple sign offs.

[c4-sponsor]

PowVT marked the issue as disagree with severity

[PowVT]

QA

[c4-judge]

0xean changed the severity to QA (Quality Assurance)

[c4-judge]

0xean marked the issue as grade-b

[c4-judge]

0xean marked the issue as grade-a