code-423n4 / 2021-12-amun-findings

0 stars 0 forks source link

Frontrunning attack via swap token functionality #284

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

thank_you

Vulnerability details

Impact

Amun utilizes both Pangolin and Uniswap's Routers to swap tokens within a given pair. One of the router functions used by Amun is swapExactTokensForTokens. Amun provides this function several arguments that the Router contract then utilizes to commence a trade between two select tokens. Documentation on Uniswap's Router swapExactTokensForTokens function can be found here. Pangolin lacks documentation on the swapExactTokensForTokens function but it is possible to look at their source code to find this same function. That can be found here.

Regardless if the Router is Uniswap or Pangolin, the swapExactTokensForTokens requires a parameter called amountOutMin. This parameter is used by the Router to calculate what the minimum amount of tokens received is acceptable in the swap. In cases where amountOutMin is set to zero, then Amun is willing to accept any amount of tokens in the trade.

For frontrunners, this is an incredible win. A front-run attack consists of a user/bot watching the Eth mempool for favorable transactions. For example, a favorable transaction for a frontrunner can occur when swapExactTokensForTokens is called and the amountOutMin argument is set to zero or an arbitrary number not based on a spot price. Let's see how this would look to the frontrunner watching for Amun contract transactions:

  1. Frontrunner spots an Amun transaction in the mempool that in-turn calls swapExactTokensForTokens with an amountOutMin set to zero. The Amun transaction is about to swap Token A for Token B. The Amun transaction is willing to accept any amount of Token B in the trade.
  2. Frontrunner creates a transaction containing a higher fee for miners guaranteeing that their transaction will run before Amun's transaction. The frontrunner's transaction buys up as much of Token B as possible from the Token A/B Pair contract. This inflates the price of Token B.
  3. The Amun transaction is executed and Amun trades Token A for Token B, further inflating the price of Token B.
  4. The frontrunner creates a final transaction that sells off all of their Token B and profits due to the artificial inflation caused by the frontrunner's first transaction and Amun's following transaction.

This attack impacts users of Amun and Amun itself. In the former case, users will be using these vulnerable contract functions to invest in Amun's basket. In the latter case, Amun's rebalance manager will utilize the RebalanceManagerV3#rebalance to rebalance the basket. Any time either the user or Amun calls these functions, a frontrunner can execute the sandwich attack and gain profit at the victim's expense through inflating prices and receiving a profit from the inflation after the victim's trade.

Proof of Concept

Amun utilizes the swapExactTokensForTokens in multiple contracts. Each of these contracts set the amountOutMin to zero or utilizes the getAmountsOut to determine the price of a token, which is equally exploitable. Because of this, a frontrunner will watch for any of these contract functions to be called, analyze the tokens that are to be traded, and commit a sandwich attack mentioned in the Impact section. Below are a list of functions vulnerable to this attack:

  1. SingleNativeTokenExit#exitEth sets the amountOutMin to zero.
  2. SingleNativeTokenExitV2#_exit which is called by both SingleNativeTokenExitV2#exit and SingleNativeTokenExitV2#exitEth. amountOutMin is set to zero.
  3. SingleTokenJoin#_joinTokenSingle estimates the amountOutMin based on the Router's getAmountsOut function. This function is equally vulnerable to the frontrun attack as getAmountsOut is calculated AFTER the frontrunner has artifically raised the price of the token Amun is receiving. getAmountsOut utilizes a spot price that is not weighted by time or ticks.
  4. SingleTokenJoinV2#_joinTokenSingle sets the amountOutMin to zero.
  5. RebalanceManagerV3#rebalance calls an internal function called _swapUniswapV2 and passes as the third argument the value 0, which then acts as the amountOutMin argument when calling swapExactTokensForTokens inside _swapUniswapV2.

Tools Used

N/A

Recommended Mitigation Steps

When utilizing swapExactTokensForTokens, Amun should utilize a price oracle to determine the accurate price before a swap can take place. Otherwise a frontrunner can frontrun the trade.

Special consideration must be taken into where that price oracle comes from. Oracles such as Chainlink can provide more secure price reporting. However, any protocol utilizing price oracles should always spend extra effort to ensure that a hacker can manipulate a price oracle in their favor.

loki-sama commented 2 years ago

All mentioned functions have a minimumReturn value, it is true that when it is set to zero or to low the trades can be front run. However the minimumReturn should not be set to a to low value similar to how uniswaps minimumReturn should not be set to zero.

0xleastwood commented 2 years ago

While I appreciate the detailed submission, the sponsor is correct, all instances of swapExactTokensForTokens, the output amount is actually checked indirectly through the struct provided in the initial function call.

So for that reason, I must mark this invalid.

0xleastwood commented 2 years ago

The only occurrence I see where this might be an issue is SingleTokenJoin#_joinTokenSingle. However, in order to sandwich attack this, an attacker cannot use a flash loan as they cannot initiate the call joinTokenSingle. So they must artificially inflate the price using their own funds, which is possible. But this is only the first trade. There are potentially more trades afterwards which are subsequently checked against _joinTokenStruct.outputAmount.

So the output amount is always validated to be equal to the expected output amount, hence, there is no concern of sandwich attacks.