Nouns Builder contest - 8olidity's results

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/governance/treasury/Treasury.sol#L141-L172

Vulnerability details

Impact

Unchecked Arrays in the execute Function,The lengths of _targets,_values, and _calldatas may all be different. The for loop in the code relies on _targets.

Proof of Concept

    function execute(
        address[] calldata _targets,
        uint256[] calldata _values,
        bytes[] calldata _calldatas,
        bytes32 _descriptionHash
    ) external payable onlyOwner {
        // Get the proposal id
        bytes32 proposalId = hashProposal(_targets, _values, _calldatas, _descriptionHash);

        // Ensure the proposal is ready to execute
        if (!isReady(proposalId)) revert EXECUTION_NOT_READY(proposalId);

        // Remove the proposal from the queue
        delete timestamps[proposalId];

        // Cache the number of targets
        uint256 numTargets = _targets.length;

        // Cannot realistically overflow
        unchecked {
            // For each target:
            for (uint256 i = 0; i < numTargets; ++i) {
                // Execute the transaction
                (bool success, ) = _targets[i].call{ value: _values[i] }(_calldatas[i]);

                // Ensure the transaction succeeded
                if (!success) revert EXECUTION_FAILED(i);
            }
        }

        emit TransactionExecuted(proposalId, _targets, _values, _calldatas);
    }

Tools Used

vscode

Recommended Mitigation Steps

if (numTargets != _values.length) revert PROPOSAL_LENGTH_MISMATCH();
if (numTargets != _calldatas.length) revert PROPOSAL_LENGTH_MISMATCH();