Uncontrolled Recursion Vulnerability In the `_switchBasket` function can result in DOS

[c4-bot-9]

Lines of code

https://github.com/reserve-protocol/protocol/blob/master/contracts/p1/BasketHandler.sol#L598-L624

Vulnerability details

Impact

In several aspects, the contract BasketHandler.sol use of recursive calls or iterations that could potentially run indefinitely or until the system reaches its gas limit, resulting in Denial of Service or other unwanted system performance impediments.

Appropriate measures must be taken into consideration to prevent such occurrences.

Proof of Concept

In the _switchBasket function, the _newBasket.nextBasket() is called without any safety measures. This could result in an infinite recursive loop because nextBasket is iterating over _targetNames, and it doesn't seem to have any condition to break or exit the loop.

https://github.com/reserve-protocol/protocol/blob/master/contracts/p1/BasketHandler.sol#L598-L624

    function _switchBasket() private {
        disabled = true;
        bool success = _newBasket.nextBasket(_targetNames, config, assetRegistry);
        if (success) {
            nonce += 1;
            basket.setFrom(_newBasket);
            basketHistory[nonce].setFrom(_newBasket);
            timestamp = uint48(block.timestamp);
            disabled = false;
        }
        uint256 len = basket.erc20s.length;
        uint192[] memory refAmts = new uint192[](len);
        for (uint256 i = 0; i < len; ++i) {
            refAmts[i] = basket.refAmts[basket.erc20s[i]];
        }
        emit BasketSet(nonce, basket.erc20s, refAmts, disabled);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

It is strongly recommended to put appropriate measures to prevent unintended infinite or overly long loops in functions. Such measures may include:

• Using a loop that inherently has a limit like a for loop that uses a counter.
• Ensuring and enforcing that the criteria that could break the loop actually can and do occur within a reasonable amount of iterations.
• Introducing a maximum number of iterations parameter.

Assessed type

DoS