code-423n4 / 2024-02-ai-arena-findings

4 stars 3 forks source link

`setupAirdrop()` can rug-pull the protocol #706

Open c4-bot-9 opened 8 months ago

c4-bot-9 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/Neuron.sol#L127-L133

Vulnerability details

Impact

Since admins are able to approve any amount of tokens to any particular addresses by calling `setupAirdrop() function - they can easily rug-pull the whole contract, by approving the addresses under their control.

Since protocol has multiple of admins (function adjustAdminAccess() from Neuron.sol is responsible for adding multiple of admins) - the risk of a malicious admin increases (the more users with higher privileges, the higher the risk that one of them might become malicious).

Proof of Concept

File: Neuron.sol

function setupAirdrop(address[] calldata recipients, uint256[] calldata amounts) external {
        require(isAdmin[msg.sender]);
        require(recipients.length == amounts.length);
        uint256 recipientsLength = recipients.length;
        for (uint32 i = 0; i < recipientsLength; i++) {
            _approve(treasuryAddress, recipients[i], amounts[i]);
        }
    }

By calling setupAirdrop(), user with administrative privileges. (isAdmin[msg.sender] == true) approves any particular address to spend tokens from treasuryAddress. Admin can call setupAirdrop() with a list of addresses he controls and provide amount as the whole balance of treasuryAddress. This basically means, that any admin can rug-pull the whole protocol.

  1. Admin calls setupAirdrop(). He sets recipients to a list of addresses he controls and amounts to the whole balance of treasuryAddress.
  2. Calling setupAirdrop() will approve recipients() to spend tokens from treasuryAddresses.
  3. Now, using the approved accounts, admin transfers all the funds from treasuryAddress.
  4. The protocol is rug-pulled

Tools Used

Manual code review

Recommended Mitigation Steps

Either set some maximum limit which can be used for the airdrop purpose (so that it won't be possible to approve the whole treasuryAddress balance) or require at least a few admins to start the airdrop. When more than one admin will be needed to call setupAirdrop(), the chances of the rug-pull will decrease. Furthermore, another idea to mitigate the risk of above scenario is to implement a timelock. Starting airdrop may be protected by the timelock. When someone will try to rug-pull the protocol, other users/admins will have enough time to react.

Assessed type

Rug-Pull

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #31

c4-judge commented 8 months ago

HickupHH3 changed the severity to QA (Quality Assurance)