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

4 stars 3 forks source link

new airdrop overwrites previous one #1264

Closed c4-bot-10 closed 8 months ago

c4-bot-10 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/Neuron.sol#L132

Vulnerability details

Description

The protocol has the ability to airdrop NRN to its community. The way an airdrop works is that the protocol hands out an allowance to transfer the airdropped amount of NRN from treasury:

Neuron::setupAirdrop

File: src/Neuron.sol

127:    function setupAirdrop(address[] calldata recipients, uint256[] calldata amounts) external {
128:        require(isAdmin[msg.sender]);
129:        require(recipients.length == amounts.length);
130:        uint256 recipientsLength = recipients.length;
131:        for (uint32 i = 0; i < recipientsLength; i++) {
132:            _approve(treasuryAddress, recipients[i], amounts[i]);
133:        }
134:    }

The issue is that _approve overwrites the previous approval. Hence if a user had an already existing unclaimed airdrop (approval) the new airdrop would overwrite the previous one and they would only get the latest one.

Impact

A user might not receive all their airdrops if they are not claimed fast enough.

Proof of Concept

Test in test/Neuron.t.sol:

    function testSetupAirdropSecondOverwritesFirst() public {
        address[] memory recipients = new address[](1);
        recipients[0] = vm.addr(3);
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = 2_000e18;

        // user recieves airdrop of 2_000 NRN
        _neuronContract.setupAirdrop(recipients, amounts);
        assertEq(_neuronContract.allowance(_treasuryAddress, recipients[0]), 2_000e18);

        amounts[0] = 1_000e18;

        // user receives another airdrop of 1_000 NRN
        _neuronContract.setupAirdrop(recipients, amounts);

        // since the user didn't claim in time the previous airdop is missed
        // and has only 1_000 NRN to claim (instead of 3_000)
        assertEq(_neuronContract.allowance(_treasuryAddress, recipients[0]), 1_000e18);
    }

Tools Used

Manual audit

Recommended Mitigation Steps

Consider adding to the previous approval when making airdrop.

Assessed type

Other

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)