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

3 stars 1 forks source link

An oracle can still approve an USD price after it has already disapproved the price. #491

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/main/src/managers/LockManager.sol#L177-L200

Vulnerability details

Impact

An oracle can approve an USD price even if it has disapproved the price already.

Proof of Concept

When an oracle approves an USD price, function approveUSDPrice only check if the oracle has approved the price already, and reverts the tx if it has. But the function does not check if the oracle has disapproved the price already. As a result, the oracle can first disapprove the price, and then approve the price again. This may mess up the pricing of tokens.

177:    function approveUSDPrice(
178:        uint256 _price
179:    )
180:        external
181:        onlyOneOfRoles(
182:            [
183:                Role.PriceFeed_1,
184:                Role.PriceFeed_2,
185:                Role.PriceFeed_3,
186:                Role.PriceFeed_4,
187:                Role.PriceFeed_5
188:            ]
189:        )
190:    {
191:        if (usdUpdateProposal.proposer == address(0)) revert NoProposalError();
192:        if (usdUpdateProposal.proposer == msg.sender)
193:            revert ProposerCannotApproveError();
194:@>      if (usdUpdateProposal.approvals[msg.sender] == _usdProposalId)
195:            revert ProposalAlreadyApprovedError();
196:        if (usdUpdateProposal.proposedPrice != _price)
197:            revert ProposalPriceNotMatchedError();
198:
199:        usdUpdateProposal.approvals[msg.sender] = _usdProposalId;
200:        usdUpdateProposal.approvalsCount++;

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

Tools Used

VS Code

Recommended Mitigation Steps

In function approveUSDPrice, check if the oracle has disapproved the USD price, and revert the tx if it has.

    function approveUSDPrice(
        uint256 _price
    )
        ...
    {
        ...
        if (usdUpdateProposal.approvals[msg.sender] == _usdProposalId)
            revert ProposalAlreadyApprovedError();
+       if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId)
+           revert ProposalAlreadyDisapprovedError();

Assessed type

Other

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory