`msg.value ` is not handled properly in `execute` function

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/main/core/solidity/contracts/governance/GovernorCharlie.sol#L309

Vulnerability details

Impact

msg.value is not handled properly in execute function . This can lead to certain scenarios : 1 . Proposals can be executed without the proposer paying anything 2 . Proposer paying more then amount required but not getting the excess Eth back 3 . Eth getting stuck in the contract

Proof of Concept

The execute function looks like this :

 function execute(uint256 _proposalId) external payable override {//@note why this is payable ??
    if (state(_proposalId) != ProposalState.Queued) revert GovernorCharlie_ProposalNotQueued();
    Proposal storage _proposal = proposals[_proposalId];
    _proposal.executed = true;
    for (uint256 _i = 0; _i < _proposal.targets.length; _i++) {
      this.executeTransaction{value: _proposal.values[_i]}(
        _proposal.targets[_i], _proposal.values[_i], _proposal.signatures[_i], _proposal.calldatas[_i], _proposal.eta
      );
    }
    emit ProposalExecutedIndexed(_proposalId);
    emit ProposalExecuted(_proposalId);
  }

But this function doesnot handle the sent ether properly . This may introduce unwanted events .

Tools Used

manual review

Recommended Mitigation Steps

implement proper logic to track and manage msg.value .

Assessed type

Invalid Validation

[c4-pre-sort]

minhquanym marked the issue as low quality report

[c4-judge]

dmvt marked the issue as unsatisfactory: Insufficient quality

[dmvt]

No impact described