All transactions with Ether to NounsDAOExecutor::executeTransaction() function will fail.

[code423n4]

Lines of code

https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/NounsDAOExecutor.sol#L173

Vulnerability details

Impact

All transactions with Ether to the NounsDAOExecutor::executeTransaction() function will fail because it does not have the payable keyword.

Proof of Concept

The executeTransaction() function of the NounsDAOExecutor contract does not have the payable keyword so every transaction with Ether to the function will fail.

The executeTransaction() has call value without a payable keyword.

function executeTransaction(address target, uint256 value, string memory signature, bytes memory data, uint256 eta)
        public
        returns (bytes memory)
    {

        ....//omitted code.
        //@audit not payable
        // solium-disable-next-line security/no-call-value
        (bool success, bytes memory returnData) = target.call{value: value}(callData); 
       //@audit no msg.value validation
        require(success, "NounsDAOExecutor::executeTransaction: Transaction execution reverted.");

        emit ExecuteTransaction(txHash, target, value, signature, data, eta);

        return returnData;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding the payable keyword to the executeTransaction() function.

Assessed type

Payable

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[eladmallel]

Wrong analysis, our executor holds any ETH we need for proposal execution. We haven't needed execute functions to be payable to date, after ~2 years of successful operation.

[c4-sponsor]

eladmallel marked the issue as sponsor disputed

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Invalid