HildisviniOttar/thorchain_vulnerability_synth.md

## thorchain_vulnerability_synth.md

      
    Raw
  

              thorchain_vulnerability_synth.md
            
          
    Synth Vulnerability: Upgrading cheap synths to RUNE native

Description

The handler_deposit.go assumes that msg.Tx.Coins[0] is of type THOR.RUNE to determine affiliate rewards.
Once synths are enabled on Chaosnet, an attacker is able to convert a synth asset worth less than THOR.RUNE to THOR.RUNE native which can be swapped for ETH, BTC (etc).
This process can be repeated until small value asset pool(s) are drained.
Attack


Attacker buys 100 BNB.BUSD and does ADD then mints synth BNB/BUSD.
Attacker craft MsgDeposit transaction swapping BNB/BUSD synth to something with cheap transaction fees e.g. BNB, with (ideally) 100% affiliate fee to attackers RUNE address. Sign and post to /txs endpoint. For example:

{
    "account_number": "588",
    "chain_id": "thorchain",
    "fee": {
        "amount": [],
        "gas": "10000000"
    },
    "memo": "",
    "msgs": [
        {
            "type": "thorchain/MsgDeposit",
            "value": {
                "coins": [
                    {
                        "amount": "10000000000",
                        "asset": "BNB/BUSD"
                    }
                ],
                "memo": "SWAP:<any asset>:<any address>:<attacker thor address>:<highest affiliate amount possible>",
                "signer": "thor....."
            }
        }
    ],
    "sequence": "6"
}

Attackers swap is subtracted to zero.
Due to a bug in handler_deposit.go:997, it assumes the affiliate fee is RuneNative (should be BNB/BUSD) and pays out the affiliate fee in native RUNE.
Attacker swaps Affiliate RUNE Native for more BNB.BUSD and repeats the process.
This attack is exponential in nature: the cheaper the asset compared to RUNE, the more effective. The higher the affiliate percentage is set, the more effective.
This attack will continue until noticed by the network or so much (minted) THOR.RUNE has swapped into BNB.BUSD that the BUSD pool is drained. Arbs may prolong the problem by topping up the Asset pool for cheap RUNE. The exploit limit is when the price of RUNE approaches the price of the asset (e.g. $1) or all the cheap asset pools are drained.

Funds at risk / severity

Critical - There are several pools of funds with assets worth less than 1 RUNE: $11m BUSD, $17m XRUNE, $6m USDC, $3m UST, $1m Dodo, $1m KYL, ...
Difficulty - Easy to moderate. If UI's support MsgDeposit with Synths, then is easy. Otherwise need to hand craft MsgDeposit, sign and post to endpoint.
Note:
--> Nothing is at risk now as Synths is disabled <--
Code Walkthrough

In handler_deposit.go addSwapV63:
Firstly amt is calculated as a dimensionless value based on the units given in MsgDeposit (e.g. BNB/BUSD):
amt := cosmos.ZeroUint()
if !msg.AffiliateBasisPoints.IsZero() && msg.AffiliateAddress.IsChain(common.THORChain) {
	amt = common.GetSafeShare(
		msg.AffiliateBasisPoints,
		cosmos.NewUint(10000),
		msg.Tx.Coins[0].Amount,
	)
	msg.Tx.Coins[0].Amount = common.SafeSub(msg.Tx.Coins[0].Amount, amt)
}
For example if we send in 100 BNB/BUSD synth, amt is 100.

For 100% affiliate fee, msg.Tx.Coins[0].Amount is subtracted to zero. The swap then does nothing.
The bug is sending the reward to the Affiliate:
coin := common.NewCoin(common.RuneNative, amt)  // BUG: ASSUMES REWARD IS THOR.RUNE 
sdkErr := h.mgr.Keeper().SendFromModuleToAccount(ctx, AsgardName, to_address, common.NewCoins(coin))
The affiliate receives amt 100 common.RuneNative (THOR.RUNE). Thus they just upgraded 100 BNB/BUSD to 100 THOR.RUNE.
The ValidateBasic checks in MsgDeposit only ensure the Coins asset is IsNative() which checks:
c.Asset.GetChain().Equals(THORChain)
So any asset on THORChain including Synths can be exploited.
Fix

The affiliate fee must be paid in the inbound asset: Tx.Coins[0].Asset, not hard coded RuneNative.
This needs to be fixed before enabling synths.
Note: The handler_observed_txin.go is not susceptible to this bug because it does a full swap from the affiliate amount/type to rune native, which correctly adjusts for pool size.
Disclosure

This was disclosed to @Heimdall at 11am 12 Aug 2021 AEST. Later to @leena.
This will be publicly disclosed once fixed in code and the entire network is running 100% of active nodes on the fixed code paths.