sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

PokemonAuditSimulator - `executeBatch()` could fail due to unbounded array #122

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

PokemonAuditSimulator

medium

executeBatch() could fail due to unbounded array

Summary

In TimelockController.sol the function executeBatch, could revert if the array of orders is too big.

Vulnerability Detail

To call executeBatch() first governance needs to scheduleBatch(). The issue is that scheduleBatch() could schedule much more operations, because it only emits an event in the for loop

        for (uint256 i = 0; i < targets.length; ++i) {
            emit CallScheduled(id, i, targets[i], values[i], payloads[i], predecessor, delay);
        }

Unlike executeBatch(), where it iterates and calls _execute() which is much more gas heavy

        for (uint256 i = 0; i < targets.length; ++i) {
            address target = targets[i];
            uint256 value = values[i];
            bytes calldata payload = payloads[i];
            _execute(target, value, payload);
            emit CallExecuted(id, i, target, value, payload);
        }

Impact

Some orders won't be executable and governance needs to schedule them again!

Code Snippet

        for (uint256 i = 0; i < targets.length; ++i) {//@audit could fail on multiple gas heavy orders
            address target = targets[i];
            uint256 value = values[i];
            bytes calldata payload = payloads[i];
            _execute(target, value, payload);
            emit CallExecuted(id, i, target, value, payload);
        }

Tool used

Manual Review

Recommendation

Check for length on the batch orders.