Munchables - John_Femi's results

Lines of code

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

Vulnerability details

Impact

Disapproved proposals can still be approved, leading to double voting. The impact of this is medium as it can skew the change of the price of tokenContracts, but still found in a permissioned function controlled by onlyOneOfRoles with priceFeeds.

Proof of Concept

In the disapproveUSDPrice function, the initial checks are

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 seen at https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L224-L231

We see, that approvals and disapprovals of msg.sender is checked before allowing a disapproval

But we check the same for approveUSDPrice function

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

as seen at https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L19-L200

There is no check for disapproval an can cause a feed to vote twice effectively skewing the outcome of the proposal

Tools Used

Manual Review

Recommended Mitigation Steps

Add a disapproval check for approveUSDPrice function

if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId)
            revert ProposalAlreadyDisapprovedError();

Assessed type

Other