Core Contracts

Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Submission Details
Severity: high
Invalid

Off-by-one error in state() leads to incorrect proposal outcome

Description

The logic inside state() needs to be:

function state(uint256 proposalId) public view override returns (ProposalState) {
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.startTime == 0) revert ProposalDoesNotExist(proposalId);
if (proposal.canceled) return ProposalState.Canceled;
if (proposal.executed) return ProposalState.Executed;
if (block.timestamp < proposal.startTime) return ProposalState.Pending;
- if (block.timestamp < proposal.endTime) return ProposalState.Active;
+ if (block.timestamp <= proposal.endTime) return ProposalState.Active;
// ... Rest of the code
}

This is because users are allowed to vote at block.timestamp = proposal.endTime as can be seen inside castVote():

function castVote(uint256 proposalId, bool support) external override returns (uint256) {
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.startTime == 0) revert ProposalDoesNotExist(proposalId);
if (block.timestamp < proposal.startTime) {
revert VotingNotStarted(proposalId, proposal.startTime, block.timestamp);
}
@---> if (block.timestamp > proposal.endTime) {
@---> revert VotingEnded(proposalId, proposal.endTime, block.timestamp);
}
// ... Rest of the code
}

Impact

The following scenario can happen:

  • At timestamp = proposal.endTime - 1 proposal has 100 YES votes more than NO votes and quorum has been reached.

  • At timestamp = proposal.endTime two actions are performed:

    • First, Alice votes NO with her 500 votes

    • Second, Bob calls execute()

  • Due to tx ordering/higher gas-fee paid Bob's tx precedes Alice's tx in the mempool

  • state() returns this proposal as ProposalState.Succeeded and hence _queueProposal() is called successfully inside execute()

  • Alice's votes now get added to proposalVote.againstVotes

  • Now after the queueing period ends, a call needs to be made to execute() again. This time when state() is internally called by execute(), Alice's votes play a part and this check fails resulting in the proposal being DEFEATED:

// After voting period ends, check quorum and votes
ProposalVote storage proposalVote = _proposalVotes[proposalId];
uint256 currentQuorum = proposalVote.forVotes + proposalVote.againstVotes;
uint256 requiredQuorum = quorum();
// Check if quorum is met and votes are in favor
@---> if (currentQuorum < requiredQuorum || proposalVote.forVotes <= proposalVote.againstVotes) {
return ProposalState.Defeated;
}
  • So execute() reverts with error InvalidProposalState()

All users expected the proposal to go through since it was already queued, but it fails now.

Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.