Closed code423n4 closed 1 year ago
front running an accept call would require someone to send an accept transaction for a proposal that is not even on chain yet (analogous for veto)
Why would anyone try to accept a proposal that hasn't been mined?
Agree. This concern does not seem practical. Closing as invalid.
Lines of code
https://github.com/PartyDAO/party-contracts-c4/blob/d129d647796369a18e30b336e74e7d1bfc779597/contracts/party/PartyGovernance.sol#L562 https://github.com/PartyDAO/party-contracts-c4/blob/d129d647796369a18e30b336e74e7d1bfc779597/contracts/party/PartyGovernance.sol#L616 https://github.com/PartyDAO/party-contracts-c4/blob/d129d647796369a18e30b336e74e7d1bfc779597/contracts/party/PartyGovernance.sol#L526
Vulnerability details
Description
There is
accept
andveto
functions in thePartyGovernance
contract. The functions accepts theproposalId
(accept
function also acceptssnapIndex
), which does not contain any information about the proposal itself. As a result, transactions of users can be front-runned to enforce them vote (or veto, in case ofveto
function) for the proposal with the sameproposalId
he doesn't want to vote on.Proof of Concept
Someone (lets call him Bob) sends a
propose
transaction to the mempool. After that someone else (lets call him Charlie) send a transactionaccept
withproposalId
equals to the expected id of the proposal. Then the malicious user can send a transaction with higher fee that creates an proposal which have the sameproposalId
but different content. This transaction most likely will be included in the block before the transaction from Bob so Charlie will vote for the proposal he doesn't want to vote on.So, it is possible that the order of transactions will be the following:
propose
transactionaccept
transactionpropose
transactionNote, that it is not necessarily for Charlie to send
accept
transaction to mempool when Bob'spropose
transaction is not included into block to get under attack. That also can happen while chain reorg.Recommended Mitigation Steps
Use a hash of content as a unique identifier, like this is done in
execute
andcancel
functions. Then it would be impossible to manipulate or front-run transactions to the governance.