Upgraded Q -> 2 from #548 [1717590798564]

[c4-judge]

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

Low 02 User Can Approve And Dissaprove

Summary

There are two functions responsible for approving and disapproving proposals. Users are expected to either approve or disapprove a proposal, but not both. Currently approve function doesn't check if the user was already disapproved.

Vulnerability Detail

In the LockManager contract, there are two functions for managing proposals: approveUSDPrice and disapproveUSDPrice. Users are expected to either approve or disapprove a proposal, but not both.

Disapprove Function:

The disapproveUSDPrice function correctly checks if the user has already either approved or disapproved the proposal. If either condition is true, the function reverts to prevent the user from performing conflicting actions.

Approve Function:

The current implementation of the approveUSDPrice function only checks if the user has already approved the proposal. It does not verify if the user has previously disapproved the proposal, creating an inconsistency.

This omission allows a user who has disapproved a proposal to later approve it, which contradicts the intended logic and can lead to inconsistent proposal states.

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

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

Impact

The lack of a disapproval check in the approveUSDPrice function can result in a proposal being both approved and disapproved by the same user, leading to inconsistencies.

Code Snippet

None

Tool used

Manual Review

Recommendation

Add this line of code tot LockManager:approveUSDPrice function:

if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId) 
    revert ProposalAlreadyDisapprovedError();

[c4-judge]

alex-ppg marked the issue as duplicate of #495

[c4-judge]

alex-ppg marked the issue as partial-25