Munchables - RotiTelur's results

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L177-L242

Vulnerability details

Vulnerability details

A user with the pricefeed_n role can disapprove a proposal and then approve it again.

Impact

Voting Manipulation: The ability to change votes can lead to manipulation of the voting process. Governance Instability: Governance decisions may become unstable if votes can be easily changed. Increased Contract Complexity: Allowing vote changes adds unnecessary complexity to the contract logic.

Proof of Concept

There are 5 addresses with the pricefeed_n role. One of these addresses can disapprove a proposal and then approve it again, leading to the issues mentioned.

Tool use Manual

Recommended Mitigation Steps

function approveUSDPrice(uint256 _price) external onlyOneOfRoles([...]) {
   if (usdUpdateProposal.proposer == address(0)) revert NoProposalError();
   if (usdUpdateProposal.proposer == msg.sender) revert ProposerCannotApproveError();
   if (usdUpdateProposal.approvals[msg.sender] == _usdProposalId) revert ProposalAlreadyApprovedError();
+++   if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId) revert ProposalAlreadyDisapprovedError();
   if (usdUpdateProposal.proposedPrice != _price) revert ProposalPriceNotMatchedError();


   usdUpdateProposal.approvals[msg.sender] = _usdProposalId;
   usdUpdateProposal.approvalsCount++;


   if (usdUpdateProposal.approvalsCount >= APPROVE_THRESHOLD) {
       _execUSDPriceUpdate();
   }


   emit ApprovedUSDPrice(msg.sender);
}

Assessed type

Other