Malt Finance contest - gzeon's results

Handle

gzeon

Vulnerability details

Impact

Timelock governor can change delay and gracePeriod at will, which render the timelock useless.

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Timelock.sol#L68

  function setDelay(uint256 _delay)
    public
    onlyRole(GOVERNOR_ROLE, "Must have timelock role")

function setGracePeriod(uint256 _gracePeriod)
    public
    onlyRole(GOVERNOR_ROLE, "Must have timelock role")

Recommended Mitigation Steps

Make delay and gracePeriod constant, when change is needed deploy a new timelock and transfer ownership.

Handle

gzeon

Vulnerability details

Impact

According to https://github.com/code-423n4/2021-11-malt#high-level-overview-of-the-malt-protocol

When the Malt price TWAP drops below a specified threshold (eg 2% below peg) then the protocol will revert any transaction that tries to remove Malt from the AMM pool (ie buying Malt or removing liquidity). Users wanting to remove liquidity can still do so via the UniswapHandler contract that is whitelisted in recovery mode.

However, in https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/DexHandlers/UniswapHandler.sol#L236 liquidity removed is directly sent to msg.sender, which would revert if it is not whitelisted https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/PoolTransferVerification.sol#L53

Recommended Mitigation Steps

Liquidity should be removed to UniswapHandler contract, then the proceed is sent to msg.sender

Handle

gzeon

Vulnerability details

Impact

maltMarketPrice in UniswapHandle return incorrect decimals of price when rewardDecimals < maltDecimals

    } else if (rewardDecimals < maltDecimals) {
      uint256 diff = maltDecimals - rewardDecimals;
      price = (rewardReserves.mul(10**diff)).mul(10**rewardDecimals).div(maltReserves);
      decimals = maltDecimals;

The correct decimals given the price formula should be rewardDecimals.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/DexHandlers/UniswapHandler.sol#L104

Recommended Mitigation Steps

Changing price formula and keep using maltDecimals is desired to preserve more decimal, i.e. change L104 to

price = (rewardReserves.mul(10**diff)).mul(10**maltDecimals).div(maltReserves);

Handle

gzeon

Vulnerability details

Impact

When malt is under-peg and the swing trader module do not have enough capital to buy back to peg, a Dutch auction is triggered to sell arb token. The price of the Dutch auction decrease linearly toward endprice until _endAuction() is called. https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L589

_endAuction() is called in

1. When auction.commitments >= auction.maxCommitments https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L212

2. On stabilize() -> checkAuctionFinalization() -> _checkAuctionFinalization() https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/StabilizerNode.sol#L146 https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L754

3. On stabilize() ->_startAuction() -> triggerAuction() -> _checkAuctionFinalization() https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/StabilizerNode.sol#L170 https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L754

It is possible manipulate the dutch auction by preventing _endAuction() being called.

Proof of Concept

Consider someone call purchaseArbitrageTokens with auction.maxCommitments minus 1 wei, _endAuction won't be called because auction.commitments < auction.maxCommitments. Further purchase would revert because purchaseAndBurn (https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L184) would likely revert since swapping 1 wei in most AMM will fail due to rounding error. Even if it does not revert, there is no incentive to waste gas to purchase 1 wei of token.

As such, the only way for the auction to finalize is to call stabilize(). However, this is not immediately possible because it require block.timestamp >= stabilizeWindowEnd where stabilizeWindowEnd = block.timestamp + stabilizeBackoffPeriod stabilizeBackoffPeriod is initially set to 5 minutes in the contract

After 5 minute, stabilize() can be called by anyone. By using this exploit, an attacker can guarantee he can purchase at (startingPrice+endingPrice)/2 or lower, given the default 10 minute auctionLength and 5 minute stabilizeBackoffPeriod. (unless a privileged user call stabilize() which override the stability window)

Also note that stabilize() might not be called since there is no incentive.

Recommended Mitigation Steps

1. Incentivize stabilize() or incentivize a permission-less call to _endAuction()
2. Lock-in auction price when user commit purchase

Handle

gzeon

Vulnerability details

Impact

One of the innovative feature of Malt is to block buying while under peg. The buy block can be bypassed by swapping to the whitelisted UniswapHandler, and then extract the token by abusing the add and remove liquidity function. This is considered a high severity issue because it undermine to protocol's ability to generate profit by the privileged role as designed and allow potential risk-free MEV.

Proof of Concept

1. User swap dai into malt and send malt directly to uniswapHandler, this is possible becuase uniswapHandler is whitelisted swapExactTokensForTokens(amountDai, 0, [dai.address, malt.address], uniswapHandler.address, new Date().getTime() + 10000);
2. User send matching amount of dai to uniswapHandler
3. User call addLiquidity() and get back LP token
4. User call removeLiquidity() and get back both dai and malt

Recommended Mitigation Steps

According to documentation in https://github.com/code-423n4/2021-11-malt#high-level-overview-of-the-malt-protocol

Users wanting to remove liquidity can still do so via the UniswapHandler contract that is whitelisted in recovery mode. , this should be exploitable. Meanwhile the current implementation did not actually allow remove liquidity during recovery mode (refer to issue "Unable to remove liquidity in Recovery Mode") This exploit can be mitigated by disabling addLiquidity() when the protocol is in recovery mode