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

3 stars 1 forks source link

Upgraded Q -> 2 from #545 [1717590716526] #558

Closed c4-judge closed 4 months ago

c4-judge commented 4 months ago

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

[L-08] Incorrect validation in function approveUSDPrice() allows price feed roles to both approve and disapprove for a proposal

Link to instance

Unlike the disapproveUSDPrice() function which validates both the disapproval and approval mapping with these checks, the approveUSDPrice() function does not check the disapproval mapping.

This allows price feed roles to first call disapproveUSDPrice(), which will pass since there has neither been an approval or disapproval from their side on the proposal yet. The function would set the disapproval mapping from price feed to the current _usdProposalId. The price feed role can then call approveUSDPrice() and pass this check since it only checks for approval mapping and not the disapproval mapping as well.

Due to this, price feed roles can both approve and disapprove for a proposal, thereby double counting their votes.

Solution: Add the below check to the approveUSDPrice() function.

File: LockManager.sol
233:         if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId)
234:             revert ProposalAlreadyDisapprovedError();
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