Price system relies on 3 out of 5 feeds reporting correct price
However, it can be
Price updates have the following lifecycle:
One of the five whitelisted Feeds proposes a price if there's no pending proposal.
Other feeds call approve or disapprove with the same price
Once a proposal reaches approval threshold (3/5), it is executed and deleted; if instead it reaches disapproval threshold (3/5), it is deleted without any price changes.
Because the only way for Feeds to approve a proposal is by its price, the following situation may occur:
But if two proposals with the same price (but for different tokens) are proposed, Feeds may unintentionally approve the latter, while meaning to approve the former proposal.
Feeds are validated by checking price against the proposed price
Feeds may approve/disapprove for a different set of contracts than they intended
Hence, two malicious feeds is enough to change the price.
Proof of Concept
M1, M2 - malicious feeds; H1,H2,H3 - honest feeds
M1 proposes a correct price for a set of tokens P1
H1 approves
H2 approves. M2 frontruns H2 by 1) approving P1, which would trigger the execution and delete the proposal; 2) creating proposal P2 with the same price but for different set of tokens. H2's txn is minted
approve/disapprove price => threshold is reached => proposal is deleted
propose the same price for a different set of contracts
Feed D approves price for set2
Recommended Mitigation Steps
Validate approvals/disapprovals by id and price, instead of only by price.
c4-bot-3
commented
1 month ago
Lines of code
https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L176-L242
Vulnerability details
Impact
Price system relies on 3 out of 5 feeds reporting correct price
However, it can be
Price updates have the following lifecycle:
proposesapriceif there's no pendingproposal.approveordisapprovewith the samepriceBecause the only way for Feeds to approve a proposal is by its
price, the following situation may occur:But if two proposals with the same price (but for different tokens) are proposed, Feeds may unintentionally approve the latter, while meaning to approve the former proposal.
Feeds are validated by checking price against the proposed price
Feeds may approve/disapprove for a different set of contracts than they intended
Hence, two malicious feeds is enough to change the price.
Proof of Concept
M1, M2 - malicious feeds; H1,H2,H3 - honest feeds
M1 proposes a correct price for a set of tokens P1
H1 approves
H2 approves. M2 frontruns H2 by 1) approving P1, which would trigger the execution and delete the proposal; 2) creating proposal P2 with the same price but for different set of tokens. H2's txn is minted
approve/disapprove price => threshold is reached => proposal is deleted
propose the same price for a different set of contracts
Feed D approves price for set2
Recommended Mitigation Steps
Validate approvals/disapprovals by id and price, instead of only by price.
Assessed type
Other