Malt Finance contest - leastwood's results

Handle

leastwood

Vulnerability details

Vulnerability details

Impact

The purchaseAndBurn function in LiquidityExtension is called by the Auction contract when users purchase arbitrage tokens. An attacker can monitor the blockchain for calls to this function and launch a sandwich attack to steal funds, assuming the attacker is a well-funded actor.

A malicious user is potentially able to steal rewards from users. As such, this issue should be considered medium risk as this results in loss of funds, but requires the attacker to be well-funded.

Proof of Concept

Consider the following scenario:

• An attacker buys Malt tokens from the uniswap pool.
• They frontrun a user calling purchaseArbitrageTokens.
• The user buys Malt tokens at an inflated price.
• The attacker sells their Malt tokens and profits.

As shown above, attackers are able to siphon funds from users overtime.

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol#L184 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/LiquidityExtension.sol#L124 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DexHandlers/UniswapHandler.sol#L131-L158

Tools Used

Manual code review Referenced Mochi review.

Recommended Mitigation Steps

Consider performing slippage checks when users purchase arbitrage tokens by calling purchaseArbitrageTokens. This can be done by simply performing a lower bounds check on the output amount received when exchanging rewards tokens for Malt tokens. This change would need to be added into the purchaseAndBurn function in LiquidityExtension.

Handle

leastwood

Vulnerability details

Impact

The notSameBlock modifier is added to sensitive functions where flashloans may put the protocol at risk. Malt Finance has preemptively addressed these concerns by limiting msg.sender from calling a set of functions multiple times within a single block. However, this can be bypassed if a flashloan attacker uses multiple contracts to act as separate EOA accounts. Therefore, notSameBlock provides security by obscurity and should be replaced with a fully fledged solution.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Permissions.sol#L135-L141

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider checking that tx.origin == msg.sender in order to prevent contracts from calling such sensitive functions. However, careful consideration needs to be made as smart wallets won't be able to call the affected functions. It is also important to note that tx.origin can add additional security concerns if not implemented correctly.

Handle

leastwood

Vulnerability details

Impact

The permissionless stabilize function in StabilizerNode is called to correct deviations in the Malt token price. When the price of Malt has appreciated above its peg, the function simply distributes rewards to LP token holders, effectively diluting the total Malt token supply. As the _distributeSupply function interacts with a Uniswap pool without performing slippage checks, an attacker can call this function and launch a sandwich attack in combination with a flash loan to steal funds.

A malicious user is potentially able to steal rewards that would otherwise be distributed to the protocol's users. As such, this issue should be considered high risk as this results in loss of funds.

Proof of Concept

Consider the following scenario:

• An attacker flashloans and buys rewards tokens from the uniswap pool.
• The attacks calls the stabilize function in StabilizerNode.
• The StabilizerNode contract buys rewards tokens at an inflated price.
• The attacker sells their rewards tokens and pays back their flashloan.

As shown above, attackers are able to siphon funds from users overtime.

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/StabilizerNode.sol#L236 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DexHandlers/UniswapHandler.sol#L160-L183

Tools Used

Manual code review Referenced Mochi review.

Recommended Mitigation Steps

Consider performing slippage checks within the _distributeSupply function. This can be done by simply performing a lower bounds check on the output amount received when exchanging Malt tokens for rewards tokens.

Handle

leastwood

Vulnerability details

Impact

purchaseArbitrageTokens enables users to commit collateral tokens and in return receive arbitrage tokens which are redeemable in the future for Malt tokens. Each auction specifies a commitment cap which when reached, prevents users from participating in the auction. However, realCommitment can be ignored by directly sending the LiquidityExtension contract collateral tokens and subsequently calling purchaseArbitrageTokens.

Proof of Concept

Consider the following scenario:

• An auction is currently active.
• A user sends collateral tokens to the LiquidityExtension contract.
• The same user calls purchaseArbitrageTokens with amount 0.
• The purchaseAndBurn call returns a positive purchased amount which is subsequently used in auction calculations.

As a result, a user could effectively influence the average malt price used throughout the Auction contract.

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol#L177-L214 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/LiquidityExtension.sol#L117-L128

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider adding a check to ensure that realCommitment != 0 in purchaseArbitrageTokens.

Handle

leastwood

Vulnerability details

Impact

_startAuction utilises the SwingTrader contract to purchase Malt. If SwingTrader has insufficient capital to return the price of Malt back to its target price, an auction is triggered with the remaining amount. However, no auction is triggered if the current auction exists, but msg.sender is still rewarded for their call to stabilize.

Proof of Concept

_shouldAdjustSupply initially checks if the current auction is active, however, it does not check if the current auction exists. There is a key distinction between the auctionActive and auctionExists functions which are not used consistently. Hence, an auction which is inactive but exists would satisfy the edge case and result in triggerAuction simply returning.

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol#L382-L386 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol#L268-L272 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/StabilizerNode.sol#L342-L344 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol#L873-L888

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider using auctionExists and auctionActive consistently in StabilizerNode and Auction to ensure this edge case cannot be abused.

Handle

leastwood

Vulnerability details

Impact

_calculateMaltRequiredForExit in AuctionEscapeHatch currently uses Malt's spot price to calculate the quantity to return to the exiting user. This spot price simply tracks the Uniswap pool's reserves which can easily be manipulated via a flash loan attack to extract funds from the protocol.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/AuctionEscapeHatch.sol#L65-L92 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/AuctionEscapeHatch.sol#L193 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DexHandlers/UniswapHandler.sol#L80-L109 https://shouldiusespotpriceasmyoracle.com/

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider implementing/integrating a TWAP oracle to track the price of Malt.

Handle

leastwood

Vulnerability details

Impact

addLiquidity is called when users reinvest their tokens through bonding events. The RewardReinvestor first transfers Malt and rewards tokens before adding liquidity to the token pool. addLiquidity provides protections against slippage by a margin of 5%, and any dust token amounts are transferred back to the caller. In this instance, the caller is the RewardReinvestor contract which further distributes the dust token amounts to the protocol's treasury. However, the token approval for this outcome is not handled properly. Dust approval amounts can accrue over time, leading to large Uniswap approval amounts by the UniswapHandler contract.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DexHandlers/UniswapHandler.sol#L212-L214 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DexHandlers/UniswapHandler.sol#L216-L218

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider resetting the approval amount if either maltUsed < maltBalance or rewardUsed < rewardBalance in addLiquidity.

Handle

leastwood

Vulnerability details

Impact

_distributeRewards attempts to reward LP token holders when the price of Malt exceeds its price target. Malt Finance is able to being Malt back to its peg by selling Malt and distributing rewards tokens to LP token holders. An external call to Auction is made via the allocateArbRewards function. Prior to this call, the StabilizerNode approves the contract for a fixed amount of tokens, however, the allocateArbRewards function does not necessarily utilise this entire amount. Hence, dust token approval amounts may accrue from within the StabilizerNode contract.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/StabilizerNode.sol#L252-L253 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol#L809-L871

Tools Used

Manual code review

Recommended Mitigation Steps

Consider resetting the approval amount if the input rewarded amount to allocateArbRewards is less than the output amount.

Handle

leastwood

Vulnerability details

Impact

MaltDataLab integrates several MovingAverage contracts to fetch sensitive data for the Malt protocol. Primary data used by the protocol consists of the real value for LP tokens, the average price for Malt and average reserve ratios. trackMaltPrice, trackPoolReserves and trackPool are called by a restricted role denoted as the UPDATER_ROLE and represented by an EOA account and not another contract. Hence, the EOA account must consistently update the aforementioned functions to ensure the most up-to-date values. However, miners can censor calls to MaltDataLab and effectively extract value from other areas of the protocol which use stale values.

Proof of Concept

Consider the following attack vector:

• The price of Malt exceeds the lower bound threshold and hence stabilize can be called by any user.
• The _stabilityWindowOverride function is satisfied, hence the function will execute.
• The state variable, exchangeRate, queries maltPriceAverage which may use an outdated exchange rate.
• _startAuction is executed which rewards msg.sender with 100 Malt as an incentive for triggering an auction.
• As the price is not subsequently updated, a malicious attacker could collude with a miner to censor further pool updates and continue calling stabilize on every fastAveragePeriod interval to extract incentive payments.
• If the payments exceed what the UPDATER_ROLE is willing to pay to call trackMaltPrice, a user is able to sustain this attack.

This threatens the overall stability of the protocol and should be properly handled to prevent such attacks. However, the fact that MaltDataLab uses a series of spot price data points to calculate the MovingAverage also creates an area of concern as well-funded actors could still manipulate the MovingAverage contract by sandwiching calls to trackMaltPrice, trackPool and trackPoolReserves.

trackMaltPrice, trackPool, and trackPoolReserves should be added to the following areas of the code where applicable. https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Bonding.sol#L159 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Bonding.sol#L173 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Bonding.sol#L177 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol#L881 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol#L710 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/StabilizerNode.sol#L156 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/StabilizerNode.sol#L190 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/ImpliedCollateralService.sol#L105

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider adding calls to trackMaltPrice, trackPoolReserves and trackPool wherever the values are impacted by the protocol. This should ensure the protocol is tracking the most up-to-date values. Assuming the cumulative values are used in the MovingAverage contracts, then sensitive calls utilising MaltDataLab should be protected from flashloan attacks. However, currently this is not the case, rather MovingAverage consists of a series of spot price data points which can be manipulated by well-funded actors or via a flashloan. Therefore, there needs to be necessary changes made to MaltDataLab to use cumulative price updates as its moving average instead of spot price.