If a proposal is disapproved by the disapproveUSDPrice() function, and then the approveUSDPrice() function is called, there is currently no check to prevent the proposal from being reapproved. As a result, it is possible for someone to vote again and change the status from disapproved to approved.
Additionally, the counters usdUpdateProposal.disapprovalsCount++ and usdUpdateProposal.approvalsCount++ are updated incorrectly.
Proof of Concept
In disapproveUSDPrice() there are two checks implemented to prevent someone from voting twice on a proposal.
if (usdUpdateProposal.approvals[msg.sender] == _usdProposalId)
revert ProposalAlreadyApprovedError();
if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId)
revert ProposalAlreadyDisapprovedError();
Lines of code
https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L177-L243
Vulnerability details
Impact
If a proposal is disapproved by the disapproveUSDPrice() function, and then the approveUSDPrice() function is called, there is currently no check to prevent the proposal from being reapproved. As a result, it is possible for someone to vote again and change the status from disapproved to approved. Additionally, the counters
usdUpdateProposal.disapprovalsCount++
andusdUpdateProposal.approvalsCount++
are updated incorrectly.Proof of Concept
In
disapproveUSDPrice()
there are two checks implemented to prevent someone from voting twice on a proposal.https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L225-L228
but in
approveUSDPrice()
there is only one checksir 0xinsanity accepts this in Curious Private Thread
Tools Used
VsCode
Recommended Mitigation Steps
In
approveUSDPrice()
The disapproval check should also be added.Assessed type
Invalid Validation