Trail of Bits Security Audit

Today, we’re thrilled to share the results of our first major audit!

We engaged the legendary software auditing firm Trail of Bits, challenging them to find issues in our suite of libraries: Orga, Merk, Ed, and abci2. In parallel, they also audited the primary application of this software, Nomic. The full report of the audit findings is available for review on Trail of Bits Publications repository on GitHub:

Trail of Bits: Orga & Merk Security Assessment

In this post, I'll explore some of their findings and our learnings, with a focus on Turbofish's libraries -- if you'd like to read about the Nomic audit results specifically, head over here!

Background

Orga is our Tendermint-based state machine engine, the framework we use to build blockchain applications in Rust. Under the hood, Orga is powered by Merk, one of the fastest Merkle key-value stores on the planet. Check out this post for more context.

Trail of Bits is a leading cybersecurity firm, with deep expertise in both blockchain and traditional software security. They showed their extensive expertise and experience with Rust, having previously audited Rust-based blockchain projects like Drift Protocol and Solana.

Two of Trail of Bits's best and brightest, Tjaden and Anish, set out to answer the following questions (in their own words):

  • Can malicious parties cause the Orga state machine to misallocate funds?
  • Can a minority of stakeholders gain undue control of the state machine?
  • Are there DoS attack vectors that could halt the forward progress of the state machine or prevent node synchronization?
  • Are third-party protocols such as IBC or Tendermint integrated correctly?
  • Can a malicious attack spoof the existence of a node in the Merk tree?
  • Can malicious inputs cause serialization/deserialization routines to trigger a panic?
  • Do the arithmetic calculations in the system account for potential overflows, precision loss, and/or division by zero?
  • Does the state machine manage accounts, balances, signatures, and fees correctly?
  • Are the uses of cryptographic signatures, hashes, and commitments vulnerable to collisions or replays?

Spoiler-alert: no critical issues were discovered -- but the things they did find are still pretty interesting!

Let's jump into some of their findings.

Findings

Slashing of re-delegated stake is computed incorrectly

When a validator is slashed due to double-signing, any stake delegated to that validator at the time of the misbehavior must also be slashed. The orga staking module allows delegators to transfer delegated coins from one validator to another; these coins are tracked so that the proper amount can be deducted if the original validator is slashed.

Due to a miscalculation in the staking module, coins that are slashed while in the process of being re-delegated will forfeit an incorrect percentage of funds. For example, if the slash_fraction_double_sign percentage is 1/20 , the re-delegated stake will be slashed by 95% rather than the intended 5%.

This miscalculation was not detected by the unit test suite because all unit testing is done with a slash_fraction_double_sign of ½, which is the unique value x for which 1 - x = x.

This is probably the most interesting issue, for two reasons:

  1. It technically, theoretically could result in a loss of funds -- the main thing you're trying to prevent when you're working with cryptocurrency.
  2. There's a generally-applicable lesson when testing software: be careful with 0.5!

For point 1, if this situation had ever occurred in a real application, only outbound redelegations would have been affected, resulting in an incorrect slashing penalty.

On point 2: if our unit tests had used any double-sign slashing rate other than 0.5, this error would have been screamingly obvious. The most intuitive value to reach for, in this case, was uniquely bad for detecting this bug. Think of something more creative the next time a unit test requires an arbitrary number from 0-1.

State sync issues

Malicious state sync peer can cause syncing nodes to crash

Interrupted snapshots can lead to inconsistent state

Malicious state sync peer can cause a stack overflow in Merk

State sync is a great feature of Tendermint and Orga apps, allowing a totally fresh node to catch up with the network in mere seconds!

The auditors found some issues which could be exploited by malicious nodes to cause a state syncing peer to crash. These issues were resolved, and malicious node operators are back to the drawing board.

Stored IBC consensus states cannot be pruned

Orga includes an IBC module, built on ibc-rs, which enables communication with any other IBC-compatible chain (e.g. for token transfers to other IBC-compatible chains).

The auditors discovered an issue that prevented the pruning of consensus states -- the deletion attempted to remove from the string key "revision_height" instead of "revision_number-revision_height". Not dangerous, but still a good find, preventing state bloat in the long-term!

Merk proofs can be forged to claim arbitrary key/value inclusions

A malicious actor can construct a fraudulent Merk proof that demonstrates the inclusion of any arbitrary key-value pair in a target tree root. This could allow attackers to steal funds from bridges and other light clients by fraudulently claiming the existence of transactions that are not included in the genuine consensus state.

A core feature of a Merkle tree is the ability to construct partial proofs, proving the presence or absence of a node within the tree with a known root hash.

During a deep dive into Merk, the auditors discovered a way to forge existence proofs of absent values. Orga's usage of Merk isn't vulnerable to this attack, but it does violate a core promise of Merk as a standalone library and has been fixed!

Conclusion

Overall, we're extremely pleased with the outcome of this audit. The auditors worked diligently, walking us through their processes throughout the duration of the audit. They discovered hard-to-find issues at different levels of the stack, and yet nothing critical was found. Thanks to them very much for their hard work!