OperatorProposal._executeOperation() should refund excess ETH

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-party/blob/440aafacb0f15d037594cebc85fd471729bcb6d9/contracts/proposals/OperatorProposal.sol#L40-L45

Vulnerability details

Impact

There are excess ETH in OperatorProposal._executeOperation, and it should be refunded.

Proof of Concept

OperatorProposal._executeOperation runs data.operator.execute with data.operatorValue of ETH.

    data.operator.execute{ value: data.operatorValue }(data.operatorData, executionData); 

When allowOperatorsToSpendPartyEth is false, it works when data.operatorValue <= msg.value.

        if (!allowOperatorsToSpendPartyEth && data.operatorValue > msg.value) {
            revert NotEnoughEthError(data.operatorValue, msg.value);
        }

So only data.operatorValue will be used out of msg.value when allowOperatorsToSpendPartyEth is false. We should refund the excess amount.

Tools Used

Manual Review

Recommended Mitigation Steps

We should refund msg.value - data.operatorValue when allowOperatorsToSpendPartyEth is false.

[0xean]

On the fence on this one, presumably the call is made with with the correct msg.value to facilitate the transaction. Will leave open for sponsor comment, but most likely this should be QA

[c4-judge]

0xean marked the issue as primary issue

[c4-sponsor]

0xble requested judge review

[0xble]

Isn't this a duplicate of #5?

[0xean]

Yes, good catch.

[c4-judge]

0xean marked the issue as duplicate of #5

[c4-judge]

0xean marked the issue as satisfactory