Missing Validation Checks in `registerUsersForPhase` Function

[hats-bug-reporter[bot]]

Github username: @Jelev123 Twitter username: zhulien_zhelev Submission hash (on-chain): 0x798b706f9bdb346e5f63bde1d4a4de39a7c31b350596614d5439114c644a2f1e Severity: medium

Description: Description\ The registerUsersForPhase function in the contract is designed to register users with their eligible amounts for specific phases (either phase 1 or phase 4 and above). However, the function lacks essential validation checks, which can lead to various data integrity issues. Specifically, the function does not verify:

• Whether user addresses are valid (non-zero addresses).
• Whether the amounts are non-zero.
• Whether users are already registered for the specified phase.

Without these checks, the function can register invalid data, leading to potential exploits and incorrect state in the contract.

1. Proof of Concept (PoC) File

registerUsersForPhase

Recommendation

function registerUsersForPhase(uint256 _phase, address[] calldata _users, uint256[] calldata _amounts)
    external
    onlyOwner
{
    if (_users.length != _amounts.length) revert("Lengths not equal");

    for (uint256 i; i < _users.length; i++) {
        // Checks
        if (_users[i] == address(0)) revert AddressNotValid(); // Ensure the user address is valid
        if (_amounts[i] == 0) revert AmountNotValid(); // Ensure the amount is valid

        if (_phase == 1) {
            if (epoch >= 1) revert NotValid(); // Ensure the epoch is before phase 1
            if (phase1Users[_users[i]] > 0) revert AlreadyRegistered(); // Ensure the user is not already registered for phase 1
            phase1Users[_users[i]] = _amounts[i]; // Register users for phase 1
        } else if (_phase >= 4) {
            if (phase4Users[_users[i]][_phase] > 0) revert AlreadyRegistered(); // Ensure the user is not already registered for the phase
            phase4Users[_users[i]][_phase] = _amounts[i]; // Register users for phase 4 or higher
        }

        emit UserRegisteredForPhase(_phase, _users[i], _amounts[i]);
    }
}

[0xRektora]

I don't an issue with the validation, except for

Whether users are already registered for the specified phase.

Which is validated on teach phase e.g

    function _participatePhase1() internal returns (uint256 oTAPTokenID) {
        uint256 _eligibleAmount = phase1Users[msg.sender];
        if (_eligibleAmount == 0) revert NotEligible();

        // Close eligibility
        phase1Users[msg.sender] = 0;

        // Mint aoTAP
        uint128 expiry = uint128(lastEpochUpdate + EPOCH_DURATION); // Set expiry to the end of the epoch
        oTAPTokenID = aoTAP.mint(msg.sender, expiry, uint128(PHASE_1_DISCOUNT), _eligibleAmount, epoch);
    }

[Jelev123]

Thank you for your feedback. I wanted to draw a parallel to your registerUsers function, which already includes comprehensive validation checks to ensure data integrity. These checks include:

• Non-zero Addresses: Ensuring user addresses are valid.
• Non-zero Amounts: Ensuring amounts are meaningful.
• Duplicate Registrations: Ensuring users are not already registered.

For consistency and to prevent potential issues, I believe it's important to incorporate similar validation checks into the registerUsersForPhase function