Attacker Can Clog The Proposal System

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/governance/GovernorBravoDelegateMaia.sol#L174-L188

Vulnerability details

Impact:

Attacker can make the whole proposal system stuck such that new proposals won't be queued.

Proof-Of-concept:

An accepted proposal's next step is to get queued and then get executed , the bug lies in the queue process.

Inside the queue function we call the queueOrRevertInternal and inside that function here https://github.com/code-423n4/2023-05-maia/blob/main/src/governance/GovernorBravoDelegateMaia.sol#L198 we call the UniV3's timelock's function queuedTransactions to see if the transaction was already queued (Timelock contract maintains a mapping from a operation hash to bool to see if queued).

A proposal contains targets and the data to call on those targets. Exploit scenario is ->

1.) Victim sends the tx to queue his proposal , let's say the proposal has 10 entries i.e 10 sets of targets and the data to call on those.

2.) Attacker has also a proposal of 10 entries , and one of those entry is exactly the same as one of the entry in victim's.

3.) Victim now sends out the tx to queue his proposal , attacker sees this and frontruns this queue tx.

4.) Now attackers proposal is queued , and now when the victim's tx executes it reverts cause of this https://github.com/code-423n4/2023-05-maia/blob/main/src/governance/GovernorBravoDelegateMaia.sol#L198 . This is because one of the entry in both were same(The hash stores inside the mapping in UniV3 timelock is set to true for queued).

One way to mitigate can be to execute the attacker's proposal fully , but his entries cost so much that it would be non practical to do so , even though it happens attacker can front run again.

Recommendation:

Ensure the proposal system can't be bricked due to MEV attacks

Assessed type

MEV

[c4-judge]

trust1995 marked the issue as unsatisfactory: Insufficient quality

[SakshamGuruji3]

@trust1995 I don't understand why is this issue marked as invalid , I gave proper explanation on how this would happen , if two proposal hashes are same (same proposal action) then due to frontrunning as described above the victim's tx would revert ,

See this in the uniV3 timelock contract->

https://github.com/Uniswap/governance/blob/master/contracts/Timelock.sol#L23

This is a mapping which would contain the attacker's proposal action (also in the victim's proposal) and since it is already there in this mapping when victim's tx runs , the check at https://github.com/code-423n4/2023-05-maia/blob/main/src/governance/GovernorBravoDelegateMaia.sol#L198 would revert since the mapping is TRUE for the proposal action.

Plus if its invalid I would love to see why , gotta learn :)

[0xLightt]

Hey, for this to happen it would be necessary for the attacker to create and pass a proposal at exactly the same time as the victim's. If the attacker creates a proposal after the victim's, it can be queued before the attacker's proposal is live. If the attacker front-runs the proposal creation, then it can be canceled after it expires and the victim's proposal can be executed.

This is all assuming the attacker can pass proposals at his will, if this is the case, then there would be worst attack vectors. There is also the possibility for the admin address (will be a multisig in our case) to cancel proposals, so this would be the last line of defense to prevent such scenarios.