PriceFeed can first `disapproveUSDPrice` and then `approveUSDPrice`

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Vulnerability details

Other priceFeeds can approve/disapprove the price proposed by a priceFeed and to prevent a priceFeed from voting twice, there is a check in disapproveUSDPrice()

 function disapproveUSDPrice(
        uint256 _price
    )
     ...

     @>   if (usdUpdateProposal.approvals[msg.sender] == _usdProposalId)
            revert ProposalAlreadyApprovedError();
     @>  if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId)
            revert ProposalAlreadyDisapprovedError();
    ...
    }

In above code, there are 2 checks which ensures priceFeed has neither approved or disapproved the price.

But the problem is there is only 1 check in approveUSDPrice(), which only checks for priceFeed has approved but not for disapprove, allowing a priceFeed to disapprove(via disapproveUSDPrice) first and then approve(via approveUSDPrice) the price, giving an option to approve & disapprove simultaneously

  function approveUSDPrice(
        uint256 _price
    )
    ...
      @>  if (usdUpdateProposal.approvals[msg.sender] == _usdProposalId)
            revert ProposalAlreadyApprovedError();
    ...
    }

Impact

PriceFeed can approve & disapprove simultaneously

Proof of Concept

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L177C4-L207C6 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L210C4-L242C6

Tools Used

Manual Review

Recommended Mitigation Steps

Add this line in approveUSDPrice()

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

Assessed type

Error

[c4-judge]

alex-ppg marked the issue as satisfactory