ETH cannot be locked for specific proposal

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/c1c7c6201d0247f92472419ff657b570f9104565/contracts/governance/NounsDAOLogicV2.sol#L333

Vulnerability details

Impact

A proposal may have a value set that is sent in NounsDAOExecutor when the call is executed. However, the ETH balance of the NounsDAOExecutor is used when doing so. There is no way to lock / reserve ETH for a specific proposal, which can lead to erroneous behavior.

Proof Of Concept

Alice proposes to transfer 1 ETH to contract F. There are a lot of votes in favor of this proposal, so Alice (or the Nouns DAO) decides to deposit 1 ETH into the Executor contract. However, there is also a successful proposal from Bob which transfer 1 ETH to contract D. This proposal already succeeded some weeks ago, but there was not enough ETH to execute it. Now, that the executor contract has 1 ETH, execute can be called on Bob's proposal and Alice's ETH is used for a completely different purpose.

Recommended Mitigation Steps

A relatively simple fix would be to make execute payable and forward the ETH if msg.value is greater than 0. Like that, ETH could be associated with a single proposal only by sending the corresponding amount when executing.

[eladmallel]

We're ok letting proposal execution fail if the treasury has insufficient funds.