search

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

3 stars 1 forks source link

Upgraded Q -> 2 from #543 [1717590668705] #557

Closed c4-judge closed 4 months ago

c4-judge commented 4 months ago

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

[04] Function approveUSDPrice does not check whether voter (a PrideFeed) already diapproved the proposal or not

Function approveUSDPrice performs the following checks to decide whether an approval is valid/acceptable:

        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();

It checks whether the the voter already approved the proposal, but it does not check whether it previously disapproved the same proposal. Hence, it is possible for a PriceFeed to disapprove, and then approve the same proposal.

(In contract, disapproveUSDPrice does check whether the PriceFeed that is voting has previously approved and also whether it previously disapproved the proposal.)

To mitigate the issue, add an additional check to function approveUSDPrice:

        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 commented 4 months ago

alex-ppg marked the issue as duplicate of #495

c4-judge commented 4 months ago

alex-ppg marked the issue as partial-25