The price can be disapproved, but it's correct price which is intended to be approved by more than APPROVE_THRESHOLD(=3)
Proof of Concept
There are 5 roles to approve/disapprove the proposed price.
If at least 3 roles approve or disapprove to assess the proposed price. Through inspect of approve, disapprove functions, approved user can't disapprove the price again, but user who did already disapprove can approve the price again.
So it means that user feels his previous disapproving decision is not correct, and changed his assessment result as approvement, but at this time, usdUpdateProposal.disapprovals[msg.sender] is not cleared and disapprovalsCount is not deducted according to it.
So it cause disapprovalsCount is remained unchanged. If another role disapproves the price, proposed price is not accepted as result.
Lines of code
https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L177-L242
Vulnerability details
Impact
The price can be disapproved, but it's correct price which is intended to be approved by more than APPROVE_THRESHOLD(=3)
Proof of Concept
There are 5 roles to approve/disapprove the proposed price. If at least 3 roles approve or disapprove to assess the proposed price. Through inspect of approve, disapprove functions, approved user can't disapprove the price again, but user who did already disapprove can approve the price again. So it means that user feels his previous disapproving decision is not correct, and changed his assessment result as approvement, but at this time,
usdUpdateProposal.disapprovals[msg.sender]
is not cleared anddisapprovalsCount
is not deducted according to it. So it cause disapprovalsCount is remained unchanged. If another role disapproves the price, proposed price is not accepted as result.ex: role1 proposed price X -- approvalsCount = 1, disapprovalsCount = 0 role2 disapprove -- approvalsCount = 1, disapprovalsCount = 1 role3 disapprove -- approvalsCount = 1, disapprovalsCount = 2 role3 approve -- approvalsCount = 2, disapprovalsCount = 2 role4 disapprove -- approvalsCount = 2, disapprovalsCount = 3
Without role5's assessment, the price can be rejected.
Tools Used
Manual review
Recommended Mitigation Steps
add the below checker in approve() function
Assessed type
Other