Skip to content

Commit

Permalink
fix: fix vuln demonstrated by #583 (#584)
Browse files Browse the repository at this point in the history
This is a fix for the 2022-10-07 Binance vuln demonstrated in #583.

**Original fix** was simply (cosmos/iavl#582):

```
if len(pin.Left) > 0 && len(pin.Right) > 0 {
	return nil, errors.New("both left and right child hashes are set")
}
```
Our iavl functions however don't return errors. Proposing to use
`panic()` instead, as it does in other parts of this file.

---
More about the vuln, for comments and archival:

* https://twitter.com/buchmanster/status/1578879225574350848
*
https://medium.com/@Beosin_com/how-did-the-bnb-chain-exploiter-pass-iavl-proof-verification-an-in-depth-analysis-by-beosin-c925b77bc13e

---------

Co-authored-by: grepsuzette <grepsuzette@users.noreply.github.com>
Co-authored-by: Antonio Navarro Perez <antnavper@gmail.com>
Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
  • Loading branch information
4 people authored Sep 22, 2023
1 parent 8eeff73 commit b449e89
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 1 deletion.
5 changes: 5 additions & 0 deletions tm2/pkg/iavl/proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var (

//----------------------------------------

// Contract: Left and Right can never both be set. Will result in a empty `[]` roothash
type proofInnerNode struct {
Height int8 `json:"height"`
Size int64 `json:"size"`
Expand Down Expand Up @@ -62,6 +63,10 @@ func (pin proofInnerNode) Hash(childHash []byte) []byte {
err = amino.EncodeVarint(buf, pin.Version)
}

if len(pin.Left) > 0 && len(pin.Right) > 0 {
panic(fmt.Sprintf("both left and right child hashes are set"))
}

if len(pin.Left) == 0 {
if err == nil {
err = amino.EncodeByteSlice(buf, childHash)
Expand Down
103 changes: 103 additions & 0 deletions tm2/pkg/iavl/proof_forgery_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package iavl_test

import (
"encoding/hex"
"math/rand"
"strings"
"testing"

"github.com/stretchr/testify/require"

"github.com/gnolang/gno/tm2/pkg/crypto/tmhash"
"github.com/gnolang/gno/tm2/pkg/db"
"github.com/gnolang/gno/tm2/pkg/iavl"
)

func TestProofForgery(t *testing.T) {
source := rand.NewSource(0)
r := rand.New(source)
cacheSize := 0
tree := iavl.NewMutableTree(db.NewMemDB(), cacheSize)

// two keys only
keys := []byte{0x11, 0x32}
values := make([][]byte, len(keys))
// make random values and insert into tree
for i, ikey := range keys {
key := []byte{ikey}
v := r.Intn(255)
values[i] = []byte{byte(v)}
tree.Set(key, values[i])
}

// get root
root := tree.WorkingHash()
// use the rightmost kv pair in the tree so the inner nodes will populate left
k := []byte{keys[1]}
v := values[1]

val, proof, err := tree.GetWithProof(k)
require.NoError(t, err)

err = proof.Verify(root)
require.NoError(t, err)
err = proof.VerifyItem(k, val)
require.NoError(t, err)

// ------------------- FORGE PROOF -------------------

forgedPayloadBytes := decodeHex(t, "0xabcd")
forgedValueHash := tmhash.Sum(forgedPayloadBytes)
// make a forgery of the proof by adding:
// - a new leaf node to the right
// - an empty inner node
// - a right entry in the path
_, proof2, _ := tree.GetWithProof(k)
forgedNode := proof2.Leaves[0]
forgedNode.Key = []byte{0xFF}
forgedNode.ValueHash = forgedValueHash
proof2.Leaves = append(proof2.Leaves, forgedNode)
proof2.InnerNodes = append(proof2.InnerNodes, iavl.PathToLeaf{})
// figure out what hash we need via https://twitter.com/samczsun/status/1578181160345034752
proof2.LeftPath[0].Right = decodeHex(t, "82C36CED85E914DAE8FDF6DD11FD5833121AA425711EB126C470CE28FF6623D5")

rootHashValid := proof.ComputeRootHash()
verifyErr := proof.Verify(rootHashValid)
require.NoError(t, verifyErr, "should verify")

// forged proofs now should make ComputeRootHash() and Verify() panic
var rootHashForged []byte
require.Panics(t, func() { rootHashForged = proof2.ComputeRootHash() }, "ComputeRootHash must panic if both left and right are set")
require.Panics(t, func() { proof2.Verify(rootHashForged) }, "forged proof should not verify")
require.Panics(t, func() { proof2.Verify(rootHashValid) }, "verify (tentatively forged) proof2 two fails with valid proof")

{
// legit node verifies against legit proof (expected)
verifyErr = proof.VerifyItem(k, v)
require.NoError(t, verifyErr, "valid proof should verify")
// forged node fails to verify against legit proof (expected)
verifyErr = proof.VerifyItem(forgedNode.Key, forgedPayloadBytes)
require.Error(t, verifyErr, "forged proof should fail to verify")
}
{
// legit node fails to verify against forged proof (expected)
verifyErr = proof2.VerifyItem(k, v)
require.Error(t, verifyErr, "valid proof should verify, but has a forged sister node")

// forged node fails to verify against forged proof (previously this succeeded!)
verifyErr = proof2.VerifyItem(forgedNode.Key, forgedPayloadBytes)
require.Error(t, verifyErr, "forged proof should fail verify")
}
}

func decodeHex(t *testing.T, str string) []byte {
t.Helper()
if strings.HasPrefix(str, "0x") {
str = str[2:]
}
b, err := hex.DecodeString(str)
if err != nil {
t.Fatalf("unable to decode string, %v", err)
}
return b
}
13 changes: 12 additions & 1 deletion tm2/pkg/iavl/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package iavl

import (
"bytes"
"errors"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -230,7 +231,7 @@ func verifyProof(t *testing.T, proof *RangeProof, root []byte) {
}
// may be invalid... errors are okay
if err == nil {
assert.Errorf(t, badProof.Verify(root),
assert.Errorf(t, rangeProofVerify(badProof, root),
"Proof was still valid after a random mutation:\n%X\n%X",
proofBytes, badProofBytes)
}
Expand All @@ -239,6 +240,16 @@ func verifyProof(t *testing.T, proof *RangeProof, root []byte) {

// ----------------------------------------

func rangeProofVerify(rangeProof *RangeProof, root []byte) (namedError error) {
defer func() {
if e := recover(); e != nil {
namedError = errors.New(e.(string))
}
}()
namedError = rangeProof.Verify(root)
return
}

func flatten(bzz [][]byte) (res []byte) {
for _, bz := range bzz {
res = append(res, bz...)
Expand Down

0 comments on commit b449e89

Please sign in to comment.