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

3 stars 1 forks source link

approveUSDPrice function allows authorized caller to call disapproveUSDPrice function then also call approveUSDPrice function #478

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#L177 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L210

Vulnerability details

Impact

It's possible for an authorized caller to call disapproveUSDPrice and then call approveUSDPrice. This means an authorized caller can have disapproval and approval counts at the same time.

This can even lead to a situation where usd price can't be updated. For instance, if the number of authorized callers is 5. One user approves and disapproves a usd price, the four other callers approve and disapprove equally (2 for, 2 against).

With the initial user having an "approve" and "disapprove", "approve" would be 3 likewise "disapprove".

Proof of Concept

An authorized caller can call disapproveUSDPrice funtion first. Then, call approveUSDPrice function after.

Unlike in disapproveUSDPrice function, there's nothing stopping an authorized caller, who has called disappproveUSDPrice function initially, from calling approveUSDPrice function.

In essence, an authorized caller can have his "vote" count for approve usd price and disapprove usd price.

Tools Used

Manual review

Recommended Mitigation Steps

In approveUSDPrice function, also check if msg.seder has already called disapproveUSDPrice function.

Assessed type

Access Control

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory