The latest malt price can be less than the actual price target and `StabilizerNode.stabilize` will revert

[code423n4]

Lines of code

https://github.com/code-423n4/2023-02-malt/blob/main/contracts/StabilityPod/StabilizerNode.sol#L188 https://github.com/code-423n4/2023-02-malt/blob/main/contracts/StabilityPod/StabilizerNode.sol#L201-L203

Vulnerability details

Impact

StabilizerNode.stabilize will revert when latestSample < priceTarget.

Proof of Concept

In StabilizerNode.stabilize, when exchangeRate > priceTarget and _msgSender is not an admin and not whitelisted, it asserts livePrice > minThreshold. And minThreshold is calculated as follows:

    uint256 priceTarget = maltDataLab.getActualPriceTarget();

        uint256 latestSample = maltDataLab.maltPriceAverage(0);
        uint256 minThreshold = latestSample -
          (((latestSample - priceTarget) * sampleSlippageBps) / 10000);

This code snippet assumes that latestSample >= priceTarget. Although exchangeRate > priceTarget, exchangeRate is the malt average price during priceAveragePeriod. But latestSample is one of those malt prices. So latestSample can be less than exchangeRate and priceTarget, so stabilize will revert in this case.

Tools Used

Manual Review

Recommended Mitigation Steps

Use minThreshold = latestSample + (((priceTarget - latestSample) * sampleSlippageBps) / 10000) when priceTarget > latestSample.

[0xScotch]

Duplicated by #13

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

0xScotch marked the issue as sponsor confirmed

[0xScotch]

We actually do want the tx to revert when latestSample < priceTarget as that means the most recent sample in the price average feed is below peg but we are in the above peg stabilization flow in the code. However, we do not want the revert to be subtraction overflow as that looks like something went wrong. So we should handle with an explicit error.

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked the issue as selected for report