Y2k Finance contest - kv's results

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L40 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L443-L446

Vulnerability details

Unbounded Loop can lead to DOS due to out of gas.

Vulnerability Detail:

As this array can grow quite large, the transaction’s gas cost could exceed the block gas limit and make it impossible to call this function at all (see @Audit):

Impact

It can lead to DOS due to being out of gas and it will cause the transfer to revert.

Code Snippet

epochs array is defined here. We can see that it is dynamic array: https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L40

    uint256[] public epochs;

Here addresses are pushed into accounts array: https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L322

        epochs.push(epochEnd);

Here loop is unbounded, there is no upperbound, which can lead to out of gas situation. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L443-L446

    function getNextEpoch(uint256 _epoch)
        public
        view
        returns (uint256 nextEpochEnd)
    {
        for (uint256 i = 0; i < epochsLength(); i++) { //@audit Unbounded Loop
            if (epochs[i] == _epoch) {
                if (i == epochsLength() - 1) {
                    return 0;
                }

Tool used

Manual Review

Recommendations

Consider introducing a reasonable upper limit based on block gas limits and/or adding a remove method to remove elements in the array.