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

0 stars 0 forks source link

Upgraded Q -> 2 from #549 [1717590836296] #561

Closed c4-judge closed 1 month ago

c4-judge commented 1 month ago

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

4

Missing Check for Prior Disapproval in approveUSDPrice Function

The approveUSDPrice function allows price feed contracts to vote in favor of a proposed price. However, unlike the disapproveUSDPrice function, approveUSDPrice does not verify whether the voter has already voted against the current proposal. A price feed contract should only be allowed to vote on one side (approve or disapprove) of the proposal. This missing check can lead to inconsistent voting behavior and potential conflicts in the proposal process.

Affected code

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();
    // @audit - LOW should check even disapprovals
    if (usdUpdateProposal.proposedPrice != _price)
        revert ProposalPriceNotMatchedError();

    usdUpdateProposal.approvals[msg.sender] = _usdProposalId;
    usdUpdateProposal.approvalsCount++;

    if (usdUpdateProposal.approvalsCount >= APPROVE_THRESHOLD) {
        _execUSDPriceUpdate();
    }

    emit ApprovedUSDPrice(msg.sender);
}

LockManager#177-207

Potential Impact

Without a check to prevent a voter who has disapproved the proposal from approving it, the voting process can become inconsistent, leading to potential manipulation or invalid results. This could undermine the integrity of the price update mechanism.

Recommended Fix

Add a check in the approveUSDPrice function to ensure that a voter who has already disapproved the proposal cannot approve it.

c4-judge commented 1 month ago

alex-ppg marked the issue as duplicate of #495

c4-judge commented 1 month ago

alex-ppg marked the issue as partial-25