Upgraded Q -> 2 from #550 [1717590861116]

[c4-judge]

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

L4. A feeder who has already disapproved the USD price can approve it in the same proposal.

Impact

There are no checks to ensure that msg.sender has not already disapproved the price in the approveUSDPrice function. This means a feeder who has previously disapproved the same price can also approve it.

Proof of Concept

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

File: src/managers/LockManager.sol#L177
    function approveUSDPrice(
        uint256 _price
    )
        external
        onlyOneOfRoles(
            [
                Role.PriceFeed_1,
                Role.PriceFeed_2,
                Role.PriceFeed_3,
                Role.PriceFeed_4,
                Role.PriceFeed_5
            ]
        )
    {
        if (usdUpdateProposal.proposer == address(0)) revert NoProposalError();
        if (usdUpdateProposal.proposer == msg.sender) // @audit missed disapprovals checks
            revert ProposerCannotApproveError();
        if (usdUpdateProposal.approvals[msg.sender] == _usdProposalId)
            revert ProposalAlreadyApprovedError();
        if (usdUpdateProposal.proposedPrice != _price)
            revert ProposalPriceNotMatchedError();
        ...
    }

Tools Used

Manual review

Recommended Mitigation Steps

It is advised to include validation to check if it has already been disapproved.

File: src/managers/LockManager.sol#L177
    function approveUSDPrice(
        uint256 _price
    )
        external
        onlyOneOfRoles(
            [
                Role.PriceFeed_1,
                Role.PriceFeed_2,
                Role.PriceFeed_3,
                Role.PriceFeed_4,
                Role.PriceFeed_5
            ]
        )
    {
        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();
        ...
    }

[c4-judge]

alex-ppg marked the issue as duplicate of #495

[c4-judge]

alex-ppg marked the issue as partial-25