Potential Bypass of Era Consistency Check in Proposal Execution

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/plugins/governance/Governance.sol#L136-L145

Vulnerability details

Impact

The current implementation performs the startedInSameEra() check after the _execute function. This sequence creates a vulnerability where the era consistency check could be bypassed if the _execute function contains operations that can prematurely terminate execution.

This vulnerability could lead to proposals being executed in a different era than they were proposed, potentially causing:

1. Execution of proposals under unexpected protocol conditions
2. Manipulation of governance decisions
3. Inconsistent protocol state
4. Possible exploitation of outdated proposals

Proof of Concept

1. A proposal is created and approved in Era A.
2. Before execution, the protocol transitions to Era B.
3. The proposal execution is initiated.
4. The _execute function encounters one of the following scenarios:
   a. Gas exhaustion: A complex operation consumes all available gas, causing the transaction to fail before reaching the startedInSameEra() check.
   b. Non-reverting opcode error: An assembly-level operation results in an error that doesn't trigger a revert (e.g., an invalid jump destination or stack underflow).
5. Due to these scenarios, the execution terminates prematurely, bypassing the startedInSameEra() check.
6. In a subsequent transaction, the proposal execution is retried. This time, it succeeds, but executes in Era B instead of Era A, potentially leading to unintended consequences.

Tools Used

Manual Review

Recommended Mitigation Steps

Move the startedInSameEra() check to the beginning of the proposal execution process, before any actions in _execute are performed. For example: ```solidity
function executeProposal(uint256 proposalId) public { require(startedInSameEra(proposalId), "new era");
super._execute(proposalId, targets, values, calldatas, descriptionHash); }

## Assessed type

Governance

[akshatmittal]

If the transaction reverts for any reason, before or after the check, the whole transaction reverts.

[c4-judge]

thereksfour marked the issue as unsatisfactory: Insufficient proof