code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-malt/blob/main/contracts/StabilityPod/StabilizerNode.sol#L178-L182 https://github.com/code-423n4/2023-02-malt/blob/main/contracts/StabilityPod/StabilizerNode.sol#L303-L306 https://github.com/code-423n4/2023-02-malt/blob/main/contracts/StabilityPod/StabilizerNode.sol#L336-L371

Vulnerability details

Impact

An early check logic in StabilizerNode.stabilize prevents possible stabilization.

Proof of Concept

In StabilizerNode.stabilize, there is an early check logic for exchangeRate and auction state. If _shouldAdjustSupply returns false, stabilize will end without any more process.

    if (!_shouldAdjustSupply(exchangeRate, stabilizeToPeg)) {
      lastStabilize = block.timestamp;
      impliedCollateralService.syncGlobalCollateral();
      return;
    }

In StabilizerNode._shouldAdjustSupply, when exchangeRate is less than lower price, we need to buy some malt to increase the malt price. It assumes to use auction in this case and checked if auction is not started so it can start an auction.

    return
      (exchangeRate <= (priceTarget - lowerThreshold) &&
        !auction.auctionExists(auction.currentAuctionId())) ||
      exchangeRate >= (priceTarget + upperThreshold);

But in some cases the protocol don't use auction and use swingTraderManager.buyMalt only.

function _triggerSwingTrader(uint256 priceTarget, uint256 exchangeRate)
    internal
  {
    ...
359    if (purchaseAmount > preferAuctionThreshold) {
360      uint256 capitalUsed = swingTraderManager.buyMalt(purchaseAmount);
      ...

    } else {
      _startAuction(originalPriceTarget);
    }

So when stabilize and _triggerSwingTrader uses swingTraderManager.buyMalt instead of auction, stabilize logic can work without considering auction state, even when current auction is live, but the early check of stabilize blocks this opportunity.

Tools Used

Manual Review

Recommended Mitigation Steps

Do auction exist check only when starts an auction really.

c4-sponsor commented 1 year ago

0xScotch marked the issue as sponsor disputed

0xScotch commented 1 year ago

This is the desired behaviour. There should only be one active stabilization strategy at any given time. If an auction is running then the swing trader should not trigger until that auction is complete.

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid

Picodes commented 1 year ago

It makes sense to not buyMalt until the auction has ended to wait to see the effect of the auction.

code-423n4 / 2023-02-malt-findings