031.md

Clumsy Currant Canary

Medium

Users' airdrop may be penalized twice

Summary

The improper month check in AirdropDistribution:371 allows penalty update for month monthsPassed. This may cause users' airdrop penalized twice.

Root Cause

In AirdropDistribution:setPenaltyPercentages, AIRDROP_PENALTY_OPERATOR_ROLE can update some accounts' penalty percentage. From the input month parameter check , we can find out that valid input month range should between monthsPassed and 6, including monthsPassed. The problem is that users can claim monthsPassed's rewards, and we will clear $.penaltyPercentageByMonth[account][i] to 0, this means that we have penalized this monthsPassed. If the setPenaltyPercentages happens after users' claim, when they try to reclaim their airdrop next month, they have paid the last month's penalty again.

For example:

1. The time is 2025-01-15. Now the monthsPassed is 1.
2. Alice claims her airdrop for this first month and pay the current penalty.
3. The admin wants to update Alice's penalty percentage for month 1. Note: Admin thinks this is one valid input now. Admin is trusted and they will not be intend to update penalty percentage after he finds out Alice's airdrop. Considering the network congestion situation, this sequence can still happen.
4. When the time comes to 2025-02-15, monthsPassed is 2.
5. Alice claims her airdrop for the second month and need to pay the month 1's penalty again.

    function setPenaltyPercentages(
        uint256[] memory penaltyPercentages,
        address[] memory accounts,
        uint256 month
    ) external {
        uint256 monthsPassed = _calculateMonthsPassed();

        // Validate the month is within the 6-month vesting period
@>        if (month < monthsPassed || month > AIRDROP_VESTING_DURATION_IN_MONTHS) {
            revert OutOfBounds();
        }
        ...
    }
    function _computePenalty(
        AirdropDistributionStorageV0 storage $,
        uint256 totalAmount,
        address account,
        uint256 monthsPassed
    ) internal returns (uint256) {
        uint256 penaltyAmount = 0;
        uint256 oneSixthAmount =
            totalAmount.mulDiv(1, AIRDROP_VESTING_DURATION_IN_MONTHS, Math.Rounding.Ceil);

        for (uint256 i = 1; i <= monthsPassed; i++) {
            // We can get the valud if i = 7, because this is one mapping, not array.
            if ($.penaltyPercentageByMonth[account][i] == 0) {
                continue;
            } else if ($.penaltyPercentageByMonth[account][i] == BASIS_POINT_BASE) {
               ...
            } else {
                uint256 monthlyPenalty =
                    oneSixthAmount.mulDiv($.penaltyPercentageByMonth[account][i], BASIS_POINT_BASE);
                penaltyAmount += monthlyPenalty;
            }
@>            $.penaltyPercentageByMonth[account][i] = 0;
        }
        return penaltyAmount;
    }

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. The time is 2025-01-15. Now the monthsPassed is 1.
2. Alice claims her airdrop for this first month and pay the current penalty.
3. The admin updates Alice's penalty percentage for month 1.
4. When the time comes to 2025-02-15, monthsPassed is 2.
5. Alice claims her airdrop for the second month and need to pay the month 1's penalty again.

Impact

Alice has to pay for the first month's penalty twice.

PoC

N/A

Mitigation

Users can claim their airdrop in month monthsPassed. So admin role should not update penalty percentage for the month monthsPassed