Upgraded Q -> 2 from #764 [1709117693673]

[c4-judge]

Judge has assessed an item in Issue #764 as 2 risk. The relevant finding follows:

Lines of code https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L396-L397

Vulnerability details Impact Old proposals below the quorum can be executed in the future when the quorum is lowered. This may cause unwanted and unexpected effects on the ecosystem.

Proof of Concept Users can create various proposals using _possiblyCreateProposal. Proposals can be executed and deleted with finalizeBallot. Initially, canFinalizeBallot is called to check if the proposal passed the quorum. If not, canFinalizeBallot returns false, the proposal remains active, regardless of the votes, and cannot be closed.

However, this can lead to an issue where, if the quorum is lowered, some proposals that didn’t get the required number of votes can be executed upon the lowering.

Example:

Assume quorum is at 20%. Proposal to SEND_SALT to an investor’s address gets 60% YES and 40% NO but doesn’t pass the quorum threshold - 60% of total staked salt (requires 3x the quorum). After a few months or a year, users decide to lower the quorum. Vote after vote passes, lowering the quorum to 10%. Now the old proposal to SEND_SALT returns true from canFinalizeBallot as it has enough votes. A user executes the old proposal and sends the salt to the appointed address. The address is no longer active, and the salt is stuck there. This can be a common case, and it can happen with all kinds of proposals.

Tools Used Manual review.

Recommended Mitigation Steps Either set an expiration date for every proposal or modify the structure of finalizeBallot to include markBallotAsFinalized:

function finalizeBallot(uint256 ballotID) external nonReentrant {

• require(proposals.canFinalizeBallot(ballotID), "The ballot is not yet able to be finalized");

• if (proposals.canFinalizeBallot(ballotID)) { Ballot memory ballot = proposals.ballotForID(ballotID);

  if (ballot.ballotType == BallotType.PARAMETER) {
      _finalizeParameterBallot(ballotID);
  } else if (ballot.ballotType == BallotType.WHITELIST_TOKEN) {
      _finalizeTokenWhitelisting(ballotID);
  } else {
      _finalizeApprovalBallot(ballotID);
  }

• proposals.markBallotAsFinalized(ballotID); } } Assessed type Error

Lines of code https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L396-L397

Vulnerability details Impact Old proposals below the quorum can be executed in the future when the quorum is lowered. This may cause unwanted and unexpected effects on the ecosystem.

Proof of Concept Users can create various proposals using _possiblyCreateProposal. Proposals can be executed and deleted with finalizeBallot. Initially, canFinalizeBallot is called to check if the proposal passed the quorum. If not, canFinalizeBallot returns false, the proposal remains active, regardless of the votes, and cannot be closed.

However, this can lead to an issue where, if the quorum is lowered, some proposals that didn't get the required number of votes can be executed upon the lowering.

Example:

Assume quorum is at 20%. Proposal to SEND_SALT to an investor's address gets 60% YES and 40% NO but doesn't pass the quorum threshold - 60% of total staked salt (requires 3x the quorum). After a few months or a year, users decide to lower the quorum. Vote after vote passes, lowering the quorum to 10%. Now the old proposal to SEND_SALT returns true from canFinalizeBallot as it has enough votes. A user executes the old proposal and sends the salt to the appointed address. The address is no longer active, and the salt is stuck there. This can be a common case, and it can happen with all kinds of proposals.

[c4-judge]

Picodes marked the issue as duplicate of #362

[c4-judge]

Picodes marked the issue as satisfactory