1

My ERC-777 smart contract (Schnoodle, symbol SNOOD) was attacked yesterday resulting in the entire liquidity in the UniswapV2Pair token being drained (104 ETH). The attack was by way of an attacker contract that performed a series of interactions with the liquidity token and my smart contract during its creation (txn here).

By clicking the 👁 icon on the list of internal transactions, I can see the following input/output data:

Input:
0x022c0d9f000000000000000000000000000000000000000000000005a3f13b802bf25f800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000180ea08644b123d8a3f0eccf2a3b45a5820755380000000000000000000000000000000000000000000000000000000000000080

Input:
0x0902f1ac
Output:
0x000000000000000000000000000000000000000000000005a3f13b802bf25f8100000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000062ad7934

Input:
0xa9059cbb0000000000000000000000000f6b0960d2569f505126341085ed7f0342b67dae0000000000000000000000000000000000000000686558904b462764f122c4e1

Input:
0xfff6cae9

Input:
0x23b872dd0000000000000000000000000f6b0960d2569f505126341085ed7f0342b67dae000000000000000000000000273521f6582076a5fc54da9af9bfca5435ffe9ec0000000000000000000000000000000000000000686558904b462764f122c4e1

Input:
0x70a082310000000000000000000000000f6b0960d2569f505126341085ed7f0342b67dae
Output:
0x0000000000000000000000000000000000000000686558904b462764f122c4e2

I've deciphered this, and it translates to these function calls:

  • 1. UniswapV2Pair.swap(amount0Out, amount1Out, to)
    • amount0Out: 104047009087796436864 or 104.xxx...4 ETH
    • amount1Out: 0
    • to: 0x180ea08644b123d8a3f0eccf2a3b45a582075538 (message sender)
  • 2. UniswapV2Pair.getReserves()
    • OUTPUT
      • reserve0: 104047009087796436865 or 104.xxx...5 ETH
      • reserve1: 1 or 1E-18 SNOOD
      • blockTimestampLast: 1655535924
  • 3. Schnoodle.transfer(recipient, amount)
    • recipient: 0xf6b0960d2569f505126341085ed7f0342b67dae (liquidity token)
    • amount: 32308960759206669952686933217 or 32.3b SNOOD
  • 4. UniswapV2Pair.sync
  • 5. Schnoodle.transferFrom(holder, recipient, amount)
    • holder: 0xf6b0960d2569f505126341085ed7f0342b67dae (liquidity token)
    • recipient: 0x273521f6582076a5fc54da9af9bfca5435ffe9ec (attacker contract)
    • amount: 32308960759206669952686933217 or 32.3b SNOOD
  • 6. Schnoodle.balanceOf(account)
    • account: 0xf6b0960d2569f505126341085ed7f0342b67dae (liquidity token)
    • OUTPUT
      • 32308960759206669952686933217 or 32.3b SNOOD

They all happened at the same second, so they're not listed in the correct order on Etherscan. But it's clear that the output of #2 (getReserves) is used as parameter amount0Out of #1 (swap) albeit with 1 subtracted which is interesting. And the output of #6 (balanceOf) is used as parameter amount of #3 and #5 (transfer and transferFrom).

So, it seems that the attacker contract gets the SNOOD balance of the liquidity token, and the amount of ETH in the liquidity token reserves, then does the following at the same time:

  • transferFrom to transfer all SNOOD from the liquidity token to the attacker contract
  • transfer to transfer all SNOOD from the message sender to the liquidity token
  • swap to swap SNOOD for 104 ETH to the message sender

At no point does the message sender receive any SNOOD. Yet it is able to transfer SNOOD to the liquidity token. I am unable to figure this out further. I decompiled the byte code using this decompiler and the attacker contract address (0x273521F6582076a5FC54da9Af9bFca5435ffE9eC), but it's outside of my comfort zone. Could this be a reentrancy attack on the SNOOD or the UniswapV2Pair contract?

I've done some research, and I found that there was previously a reentrancy vulnerability for ERC-777 tokens using Uniswap v1. See here which links to the exploit example on GitHub, and also the Uniswap whitepaper which indicates that the vulnerability was patched in Uniswap v2.

What I would like to know is, how did this attack take place, where is the vulnerability? And how do I protect against it? If this is an vulnerability in the SNOOD code, then how do I fix it? If it's a vulnerability in the UniswapV2Pair code related to ERC-777 tokens which is what SNOOD is, then I guess I would have to change my contract to ERC-20, or use UniswapV3Pair, or another DEX altogether.

There is also a $1500 bounty (negotiable) available for solving this vulnerability which can be viewed here via our Dework platform.

Improve this question

2 Answers 2

Reset to default
1

Seems to me that your error is here. Typically this kind of computation is done using fixed-point arithmetic or using a reflection * totalSupply / reflectedSupply calculation. In the case of transaction 0x9a6227ef97d7ce75732645bd604ef128bb5dfbc1bfbe0966ad1cd2870d45a20e , during the call to _spendAllowance, the _getStandardAmount call returns 0, even though the amount being transferred is significant. It's hard to tell what the intent of this code is, but the result is that the reflected amount isn't converted to a token amount, but is instead simply set to zero because _getReflectRate returns a large value. As a result, even though the UniswapV2 trading pair does not have an allowance set, the attacker is able to transferFrom the total balance of the pair to their attack contract. Subsequently, the attacker calls sync to force the pair to take up its new, vastly-decreased balance as the reserve. After that, it's a simple matter of swapping the stolen SNOOD tokens for the WETH left in the pair.

Improve this answer
10
  • 1
    To put it another way, your transferFrom is broken and lets anyone transfer anyone's tokens.
    – Duncan Townsend
    Jun 20, 2022 at 1:07
  • 1
    Yes, it would seem that it's converting the wrong way. My recommendation here is that _spendAllowance should not convert at all. Allowances are denominated in tokens, not reflection, therefore the token-denominated value passed to _spendAllowance should remain token-denominated and never be converted into a reflection
    – Duncan Townsend
    Jun 20, 2022 at 1:42
  • 1
    I recommend against doing that. Allowances should be stored token-denominated, not reflection-denominated. This probably works most of the time because most users will set "infinite" allowances so it doesn't matter. But if another DeFi protocol depends on allowances being consistent, it will break. It would be surprising if allowance increased with balances due to the reflection mechanism.
    – Duncan Townsend
    Jun 20, 2022 at 2:00
  • 1
    I arrived at my conclusion by looking at the call trace on tenderly, so yes I do see the attack I described in the logs
    – Duncan Townsend
    Jun 20, 2022 at 2:00
  • 1
    And lastly, you have a rounding error problem in _getReflectedAmount and _getStandardAmount. _getReflectedAmount should compute something like amount * super.totalSupply() / totalSupply() and _getStandardAmount should compute amount * totalSupply() / super.totalSupply(). If those overflow, use FullMath github.com/Uniswap/v3-core/blob/main/contracts/libraries/… . Once again, this mostly doesn't matter, but as we can see in the case of your use of _getStandardAmount in _spendAllowance, it can be catastrophic
    – Duncan Townsend
    Jun 20, 2022 at 2:01
-1

I'm not sure if it's useful now, but I just dug into this and found the problem.

Everything Duncan addressed is correct, but the question remains:

WHY does _getStandardAmount return zero?

The formula is correct — it's taking totalSupply() / totalSupply on ERC777 contract, which equals the # minted. Total supply divided by number minted should always be greater than or equal to 1 (as long as we haven't minted more than supply).

The answer is in the initialize() function. You set the totalSupply to equal initialTokens * 10 ** decimnals(), but then you proceed to mint MAX - (MAX % totalSupply()), which is a massive number.

As a result, the number minted WAY outweighs the totalSupply on the implementation contract, which results in the 0 return value.

Hope that's helpful.

Improve this answer
1
  • You've misunderstood the algorithm which is a reflective algorithm. _getStandardAmount does not take totalSupply() / _totalSupply. It takes reflectedAmount which is always a large amount. This is then divided by the ratio of the reflected total supply to the total supply (super.totalSupply() / _totalSupply) which is a large amount also. So the orders of magnitude of the divisor and the dividend are equivalent. However, this was not the question, and you should not be asking a question as an Answer.
    – Neo
    Aug 25, 2022 at 13:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.