Lines of code

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

Vulnerability details

Impact

A price feed can approve a USD price proposal after previously disapproving it. This inconsistency can lead to manipulation and unreliable approval processes, undermining the integrity of the price update mechanism.

Proof of Concept

In the approveUSDPrice function, there is no check to see if the caller has previously disapproved the same price proposal. This allows a price feed to both disapprove and then approve the same proposal:

function approveUSDPrice(uint256 _price) external onlyOneOfRoles([...]) {
    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();

    // Missing check for previous disapproval
    usdUpdateProposal.approvals[msg.sender] = _usdProposalId;
    usdUpdateProposal.approvalsCount++;

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

    emit ApprovedUSDPrice(msg.sender);
}

Tools Used

Manual Review

Recommended Mitigation Steps

Add a check in the approveUSDPrice function to ensure that the caller has not previously disapproved the same price proposal:

function approveUSDPrice(uint256 _price) external onlyOneOfRoles([...]) {
    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();
    if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId) revert ProposalAlreadyDisapprovedError(); // Add this line

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

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

    emit ApprovedUSDPrice(msg.sender);
}

This ensures that a price feed cannot approve a proposal after previously disapproving it, maintaining the integrity of the approval process.

Assessed type

Invalid Validation

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

Approval After Disapproval in `approveUSDPrice` Function #505