Lines of code

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

Vulnerability details

Impact

The approveUSDPrice function does not check if a user has already disapproved a proposal before approving it. This oversight allows users with the appropriate role to disapprove and then approve the same proposal, which is a security concern.

Proof of Concept

The approveUSDPrice function currently only checks if a user has approved the proposal but does not verify if they have disapproved it. As a result, users who have the role can disapprove a proposal and then approve it afterward. This loophole undermines the integrity of the approval process within the contract.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a condition to check if the user has disapproved the proposal before allowing them to approve it. Here is the suggested code modification:

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

This change ensures that users who have disapproved a proposal cannot approve it subsequently, maintaining the intended integrity of the contract's proposal approval process.

Assessed type

Governance

code-423n4 / 2024-05-munchables-findings

Anyone Can Disapprove and Then Approve #479