Price Feeds can disapprove and approve the same proposal

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Summary

The protocol utilizes five price feeds to establish consensus on the USD price of an asset. To determine the current reported price, these feeds can either disapprove or approve a proposed price. Upon reaching a predefined threshold, a disapproved price is removed, whereas an approved price is accepted. However, due to a missing validation check in the approveUSDPrice function, a single price feed can concurrently disapprove and approve the same price proposal.

Vulnerability Detail

there are 5 price Feed roles in the protocol that push the price of tokens supported by the protocol in USD, they approve and disapprove using functions approveUSDPrice and disapproveUSDPrice.

• approveUSDPrice

  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.proposedPrice != _price)
          revert ProposalPriceNotMatchedError();
  
      ...

• disapproveUSDPrice

  
  function disapproveUSDPrice(
      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.approvals[msg.sender] == _usdProposalId)
          revert ProposalAlreadyApprovedError();
      if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId)
          revert ProposalAlreadyDisapprovedError();
      if (usdUpdateProposal.proposedPrice != _price)
          revert ProposalPriceNotMatchedError();

as we can see in the `disapproveUSDPrice` it is checked that the msgSender did not previously approve or disapprove the price. but this check is lacking in the `approveUSDPrice`. allowing a priceFeed to both approve and disapprove a usd price.

## Impact

a price Feed role is allowed to approve and disapprove a price , leading to wrong threshold calculation.

## Tool used

Manual Review

## Recommendation

```diff=177
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();

Assessed type

Context

[c4-judge]

alex-ppg marked the issue as satisfactory