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

3 stars 1 forks source link

Missing check that proposal was not disapproved in the `approveUSDPrice()` function. #487

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

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

Vulnerability details

In the approveUSDPrice() function, there is no check to prevent the PriceFeed role from double voting, first disapproving and later approving the new price for the token.

A similar check is present in the disapproveUSDPrice() function to prevent double voting by the same actor.

Impact

The PriceFeed role can disapprove a new price update proposal and later approve it, resulting in double event emission that can corrupt off-chain data.

Recommended Mitigation Steps

Update the approveUSDPrice() function to include a check that ensures a proposal that has been disapproved by a voter cannot be approved by the same voter.

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

Invalid Validation

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory