Lines of code

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

Vulnerability details

Impact

For a proposal to be executed, it must receive sufficient approvals. On the other hand, the proposal could be disapproved. Only Role.PriceFeed can approve and disapprove. In the current implementation, it seems that they are limited to not giving more than one vote, but there is a way to make double voting possible. Role.PriceFeed can first disapprove and then approve, which can compromise the whole logic of the vote due to the possibility of giving 2 votes rather than the expected 1.

Proof of Concept

Lets say we have the following scenario: APPROVE_THRESHOLD=3; DISAPPROVE_THRESHOLD=3;

1.A proposal is submitted and awaits approval 2.Role.PriceFeed_1 disapproves the proposal //0 approvals - 1 disapproval 3.Role.PriceFeed_2 approves the proposal // 1 approval - 1 disapproval 4.Role.PriceFeed_3 approves the proposal // 2 approvals - 1 disapproval 5.Role.PriceFeed_4 disapproves the proposal // 2 approvals - 2 disapprovals 6.Role.PriceFeed_1 approves the proposal // 3 approval - 2 disapprovals

The proposal will be executed based on Role.PriceFeed_1 double voting (2,6).

Tools Used

Manual review

Recommended Mitigation Steps

Consider removing the possibility for double voting. In approveUSDPrice() add:

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

Assessed type

Other

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

Possible double voting potentially could manipulate USD price logic #508