If VAULT_CAP & NO_VAULT_CAP set to zero will block user stake BYTES

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1060-L1067 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1084-L1088 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1851-L1856

Vulnerability details

Impact

The function _stakeBytes is a private function in the NeoTokyoStaker.sol that handles the staking of BYTES into a Citizen by a user.

There are two conditions to check whether staking BYTES into S1 Citizen or S2 Citizen.

If the VAULT_CAP or NO_VAULT_CAP constant is set to zero or even small number, then the condition citizenStatus.stakedBytes + amount > cap will always be true, which means that users will not be able to stake any BYTES tokens into a S1 Citizen or S2 Citizen. This is because the cap variable will always be zero in this case, and any non-zero amount being staked will cause the revert.

Proof of Concept

    /**
        A private function for managing the staking of BYTES into a Citizen.
    */
    function _stakeBytes (
        uint256
    ) private {
        uint256 amount;
        uint256 citizenId;
        uint256 seasonId;
        assembly{
            amount := calldataload(0x44)
            citizenId := calldataload(0x64)
            seasonId := calldataload(0x84)
        }

        // Attempt to transfer BYTES to escrow.
        _assetTransferFrom(BYTES, msg.sender, address(this), amount);

        // Handle staking BYTES into an S1 Citizen.
        if (seasonId == 1) {
            StakedS1Citizen storage citizenStatus = stakedS1[msg.sender][citizenId];
            uint256 cap = VAULT_CAP;
            if (!citizenStatus.hasVault) {
                cap = NO_VAULT_CAP;
            }
            if (citizenStatus.stakedBytes + amount > cap) { //@audit if cap is zero, might cause revert.
                revert AmountExceedsCap(citizenStatus.stakedBytes + amount, cap);
            }

            // Validate that the caller actually staked the Citizen.
            if (citizenStatus.timelockEndTime == 0) {
                revert CannotStakeIntoUnownedCitizen(citizenId, seasonId);
            }

            PoolData storage pool = _pools[AssetType.S1_CITIZEN];
            unchecked {
                uint256 bonusPoints = (amount * 100 / _BYTES_PER_POINT);
                citizenStatus.stakedBytes += amount;
                citizenStatus.points += bonusPoints;
                pool.totalPoints += bonusPoints;
            }

        // Handle staking BYTES into an S2 Citizen.
        } else if (seasonId == 2) {
            StakedS2Citizen storage citizenStatus = stakedS2[msg.sender][citizenId];
            uint256 cap = NO_VAULT_CAP;
            if (citizenStatus.stakedBytes + amount > cap) {  //@audit if cap is zero, might cause revert.
                revert AmountExceedsCap(citizenStatus.stakedBytes + amount, cap);
            }

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1851-L1856

    function configureCaps (
        uint256 _vaultedCap,   //@audit without zero value check
        uint256 _unvaultedCap    //@audit without zero value check
    ) hasValidPermit(UNIVERSAL, CONFIGURE_CAPS) external {
        VAULT_CAP = _vaultedCap; 
        NO_VAULT_CAP = _unvaultedCap;
    }

Tools Used

Manual

Recommended Mitigation Steps

Add zero value or minimum value check in configureCaps()

[c4-judge]

hansfriese marked the issue as unsatisfactory: Out of scope

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #186

[c4-judge]

hansfriese marked the issue as unsatisfactory: Out of scope