Rapid Growth of _keyId Potentially Leads to DoS Risks

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/PowerFarms/PowerFarmNFTs/MinterReserver.sol#L63-L94 https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/PowerFarms/PowerFarmNFTs/MinterReserver.sol#L114-L139

Vulnerability details

Impact

Based on the logic in the _mintKeyForUser function, it's clear that _keyId increases regardless of whether a reservedKey is added or a mintKey (where mintKey will delete the reservedKey). If an attacker repetitively performs the operations of adding a reservedKey, minting an NFT, and then adding another reservedKey, several potential issues could arise:

1. Rapid Increase of _keyId:

Given that _keyId is calculated based on the sum of totalMinted and totalReserved, executing these operations repetitively would lead to a rapid increase in _keyId. Although reaching a very large number in itself doesn't directly affect the smart contract's security or functionality, it's a consideration for contract design.

2. Contract State Inflation:

Each time a reservedKey is added, a new entry is made in the reservedKeys mapping. If these operations are performed repetitively by an attacker, the mapping could accumulate a large number of entries, leading to an inflated contract state. This might increase transaction costs due to the higher cost of state changes and decrease efficiency when reading contract states.

3. Consumption of Contract Resources:

The repetitive addition of reserved keys and NFT minting involves executing transactions, consuming network resources and the attacker's ether (for transaction fees). While this is a potential issue for other network users (due to possible network congestion), it also represents a cost for the attacker.

4. Potential Denial of Service (DoS) Attack:

If the attacker's goal is to render the contract unusable due to excessive resource consumption, the repetitive addition and utilization of reservedKey could be part of a DoS attack strategy. By inflating the contract's state and consuming network resources, the attacker might aim to make the contract or its functions unreliable or too costly to use.

5. Unused Reserved Keys:

If a user reserves a key but never uses it to mint an NFT, that reserved key might occupy the contract state indefinitely. Over time, this could lead to state inflation, increasing transaction costs. Introducing a mechanism to deal with or clean up long-unused reserved keys might be beneficial.

Proof of Concept

The _reserveKey function in the MinterReserver project is responsible for generating a new _keyId and storing it in the reservedKey dictionary. It calls _getNextReserveKey and _incrementReserved to generate a new _keyId. A _keyId is the sum of totalMinted and the incrementally increased totalReserved.

https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/PowerFarms/PowerFarmNFTs/MinterReserver.sol#L63-L94

    function _incrementReserved()
        internal
        returns (uint256)
    {
        return ++totalReserved;
    }

    function _getNextReserveKey()
        internal
        returns (uint256)
    {
        return totalMinted + _incrementReserved();
    }

    function _reserveKey(
        address _userAddress,
        uint256 _wiseLendingNFT
    )
        internal
        returns (uint256)
    {
        if (reservedKeys[_userAddress] > 0) {
            revert AlreadyReserved();
        }

        uint256 keyId = _getNextReserveKey();

        reservedKeys[_userAddress] = keyId;
        farmingKeys[keyId] = _wiseLendingNFT;

        return keyId;
    }

The _mintKeyForUser function is responsible for minting keys. The main process is as follows:

1. Receives _userAddress and its _keyId
2. Validates that _keyId is valid
3. Deletes the _keyId stored in reservedKeys for the _userAddress
4. Mints a key using _userAddress and _keyId
5. Increments totalMinted
6. Decrements totalReserved

https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/PowerFarms/PowerFarmNFTs/MinterReserver.sol#L114-L139

    function _mintKeyForUser(
        uint256 _keyId,
        address _userAddress
    )
        internal
        returns (uint256)
    {
        if (_keyId == 0) {
            revert InvalidKey();
        }

        delete reservedKeys[
            _userAddress
        ];

        FARMS_NFTS.mintKey(
            _userAddress,
            _keyId
        );

        totalMinted++;
        totalReserved--;

        return _keyId;
    }

Following the logic of _reserveKey and _mintKeyForUser, consider the following process to observe changes in _keyId, totalMinted, totalReserved, and reservedKey:

1. user_1 adds a reserved key
2. user_2 adds a reserved key
3. user_3 adds a reserved key
4. Mint the reserved key for user_2
5. user_4 adds a reserved key

Result:

_keyId	totalMinted	totalReserved	reservedKey
1	0	1	{'user_1':1}
2	0	2	{'user_1':1,'user_2': 2}
3	0	3	{'user_1':1,'user_2': 2,'user_3': 3}
4	1	3	{'user_1':1,'user_3': 3}
5	1	4	{'user_1':1,'user_2': 2,'user_3': 3,'user_4': 5}

The observation shows that _keyId continuously increases, whether for reserved or minted keys. There are no limitations on the frequency and quantity of reserved and minted keys in this process.

Tools Used

Manual Review

Recommended Mitigation Steps

• Limit Frequency and Quantity: Implement limitations on the frequency and quantity of such operations to prevent rapid repetition.
• Set Permissions: Ensure only legitimate users and participants of the contract can perform these operations, not just any address.
• State Cleanup: Design mechanisms to clean up entries in the reservedKeys mapping when they are no longer needed, to prevent unlimited state growth.
• Reserved Key Expiry: Set an expiration for reserved keys and automatically release them when unused by the expiry, to avoid indefinite state growth.

Assessed type

DoS

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[GalloDaSballo]

GPT

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid