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 10 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.

Give us feedback!