stabilize() is vulnerable to flashloan sandwich attack

[code423n4]

Handle

WatchPug

Vulnerability details

When the price of Malt is off the lowerThreshold and upperThreshold, StabilizerNode.sol will market buy/sell Malt.

However, since the market sell can be triggered by anyone, and there is no slippage control, it makes it vulnerable to flashloan sandwich attack.

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/StabilizerNode.sol#L145-L174

function stabilize() external notSameBlock {
    auction.checkAuctionFinalization();

    require(
      block.timestamp >= stabilizeWindowEnd || _stabilityWindowOverride(),
      "Can't call stabilize"
    );
    stabilizeWindowEnd = block.timestamp + stabilizeBackoffPeriod;

    rewardThrottle.checkRewardUnderflow();

    uint256 exchangeRate = maltDataLab.maltPriceAverage(priceAveragePeriod);

    if (!_shouldAdjustSupply(exchangeRate)) {
      maltDataLab.trackReserveRatio();

      lastStabilize = block.timestamp;
      return;
    }

    emit Stabilize(block.timestamp, exchangeRate);

    if (exchangeRate > maltDataLab.priceTarget()) {
      _distributeSupply();
    } else {
      _startAuction();
    }

    lastStabilize = block.timestamp;
  }

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/StabilizerNode.sol#L211-L246

function _distributeSupply() internal {
    ...
    uint256 swingAmount = swingTrader.sellMalt(tradeSize);

    ...

    malt.mint(address(dexHandler), tradeSize);
    emit MintMalt(tradeSize);
    uint256 rewards = dexHandler.sellMalt();

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/StabilizerNode.sol#L317-L341

function _startAuction() internal {
    ...
    uint256 amountUsed = swingTrader.buyMalt(purchaseAmount);

POC

When the price is below lowerThreshold.

The attacker can:

1. Flash borrow a large amount of DAI and buy Malt in the market;
2. Call stabilize(), the swingTrader will buy Malt at a higher price;
3. Dump the Malt tokens bought before for profit.

Recommendation

Consider making stabilize() function only allowed to be called by an EOA to prevent flash loan sandwich attack.

[0xScotch]

This bug needs to be mitigated. However, the POC example given doesn't apply as under lowerThreshold would put the protocol into recovery mode where non-whitelisted buyers would be blocked.

We can add a before transfer hook on Malt that will trigger stabilize when a Malt sell is going through above upperThreshold. This will protect against the frontrunning above peg.

Guarding stabilize to only EOA is a good idea though.

[0xScotch]

#56

[GalloDaSballo]

@0xScotch I have yet to judge this finding.
However be aware that a Only-EOA check (msg.sender != tx.origin) is destined to eventually fail as eventually EIP-3074 will make it so that there's no difference between a contract and an EOA

[bitdeep]

Proposing onlyEOA via contract size check:

  function isContract(address account) public view returns (bool) {
    uint size;
    assembly {
      size := extcodesize(account)
    }
    return size > 0;
  }

[GalloDaSballo]

Proposing onlyEOA via contract size check:

  function isContract(address account) public view returns (bool) {
    uint size;
    assembly {
      size := extcodesize(account)
    }
    return size > 0;
  }

@bitdeep I believe this has been proven wrong multiple times.
Basicaly isContract (the check you're doing) works only for verifying that something is a contract.
It doesn't guarantee that something is not a contract.
That's because a contract can be written to perform operation in it's constructor, while in it's constructor, the check for size := extcodesize(account) will return 0 (because space was not yet allocated)

In summary, you can always verify if something IS a contract. And for now you can still use the msg.sender == tx.origin check, but eventually you will not have any way of proving that something is an EOA

[GalloDaSballo]

From reading the code I believe that anyone can sell MALT at any time.
This means that the token can be sold with the goal of reducing it's price.
After the price reduction a call to stabilize can be performed.
This allows the caller to get a small caller incentive as well as to trigger an auction.
The auction effectively triggers the system to sell the min(idealAmount, currenrtlyOwnerAmoutn) with the goal of bringing back the malt price to peg.

The question that this left unsolved is how would the trader be able to get malt for a discount to then dump it for again.

I think this can be merged as duplicate of #56

As such medium severity is more correct