Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: backport proof fix #582

Merged
merged 3 commits into from
Oct 8, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@ else
endif
.PHONY: install

test-short:
@echo "--> Running go test"
@go test ./... $(LDFLAGS) -v --race --short
.PHONY: test-short

test:
@echo "--> Running go test"
@go test ./... $(LDFLAGS) -v --race
@go test ./... $(LDFLAGS) -v
.PHONY: test

tools:
Expand Down
7 changes: 7 additions & 0 deletions proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ var (
)

//----------------------------------------
// ProofInnerNode
// Contract: Left and Right can never both be set. Will result in a empty `[]` roothash

type ProofInnerNode struct {
Height int8 `json:"height"`
Expand Down Expand Up @@ -76,6 +78,10 @@ func (pin ProofInnerNode) Hash(childHash []byte) ([]byte, error) {
err = encoding.EncodeVarint(buf, pin.Version)
}

if len(pin.Left) > 0 && len(pin.Right) > 0 {
return nil, errors.New("both left and right child hashes are set")
}

if len(pin.Left) == 0 {
if err == nil {
err = encoding.EncodeBytes(buf, childHash)
Expand All @@ -91,6 +97,7 @@ func (pin ProofInnerNode) Hash(childHash []byte) ([]byte, error) {
err = encoding.EncodeBytes(buf, childHash)
}
}

if err != nil {
return nil, fmt.Errorf("failed to hash ProofInnerNode: %v", err)
}
Expand Down
106 changes: 106 additions & 0 deletions proof_forgery_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package iavl_test

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

"github.com/cosmos/iavl"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/crypto/tmhash"
db "github.com/tendermint/tm-db"
)

func TestProofFogery(t *testing.T) {
source := rand.NewSource(0)
r := rand.New(source)
cacheSize := 0
tree, err := iavl.NewMutableTreeWithOpts(db.NewMemDB(), cacheSize, nil, false)
require.NoError(t, err)

// 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, err := tree.WorkingHash()
require.NoError(t, err)
// 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 := mustDecode("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 = mustDecode("82C36CED85E914DAE8FDF6DD11FD5833121AA425711EB126C470CE28FF6623D5")

rootHashValid := proof.ComputeRootHash()
verifyErr := proof.Verify(rootHashValid)
require.NoError(t, verifyErr, "should verify")
// forgery gives empty root hash (previously it returned the same one!)
rootHashForged := proof2.ComputeRootHash()
require.Empty(t, rootHashForged, "roothash must be empty if both left and right are set")
verifyErr = proof2.Verify(rootHashForged)
require.Error(t, verifyErr, "should not verify")

// verify proof two fails with valid proof
err = proof2.Verify(rootHashValid)
require.Error(t, err, "should not verify different root hash")

{
// 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 mustDecode(str string) []byte {
if strings.HasPrefix(str, "0x") {
str = str[2:]
}
b, err := hex.DecodeString(str)
if err != nil {
panic(err)
}
return b
}
8 changes: 8 additions & 0 deletions proof_iavl_absence.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,22 @@ func AbsenceOpDecoder(pop tmmerkle.ProofOp) (merkle.ProofOperator, error) {
if err != nil {
return nil, err
}

if n != len(pop.Data) {
return nil, fmt.Errorf("unexpected bytes, expected %v got %v", n, len(pop.Data))
}

pbProofOp := &iavlproto.AbsenceOp{}
err = proto.Unmarshal(bz, pbProofOp)
if err != nil {
return nil, err
}

proof, err := RangeProofFromProto(pbProofOp.Proof)
if err != nil {
return nil, err
}

return NewAbsenceOp(pop.Key, &proof), nil
}

Expand Down Expand Up @@ -87,24 +91,28 @@ func (op AbsenceOp) Run(args [][]byte) ([][]byte, error) {
if len(args) != 0 {
return nil, errors.Errorf("expected 0 args, got %v", len(args))
}

// If the tree is nil, the proof is nil, and all keys are absent.
if op.Proof == nil {
return [][]byte{[]byte(nil)}, nil
}

// Compute the root hash and assume it is valid.
// The caller checks the ultimate root later.
root := op.Proof.ComputeRootHash()
err := op.Proof.Verify(root)
if err != nil {
return nil, errors.Wrap(err, "computing root hash")
}

// XXX What is the encoding for keys?
// We should decode the key depending on whether it's a string or hex,
// maybe based on quotes and 0x prefix?
err = op.Proof.VerifyAbsence(op.key)
if err != nil {
return nil, errors.Wrap(err, "verifying absence")
}

return [][]byte{root}, nil
}

Expand Down
1 change: 1 addition & 0 deletions proof_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func (pl PathToLeaf) stringIndented(indent string) string {

// `computeRootHash` computes the root hash assuming some leaf hash.
// Does not verify the root hash.
// Contract: Caller must verify that the roothash is correct by calling `.verify()`.
func (pl PathToLeaf) computeRootHash(leafHash []byte) ([]byte, error) {
var err error
hash := leafHash
Expand Down
7 changes: 7 additions & 0 deletions proof_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,16 @@ func (proof *RangeProof) VerifyItem(key, value []byte) error {
if proof == nil {
return errors.Wrap(ErrInvalidProof, "proof is nil")
}

if !proof.rootVerified {
return errors.New("must call Verify(root) first")
}

leaves := proof.Leaves
i := sort.Search(len(leaves), func(i int) bool {
return bytes.Compare(key, leaves[i].Key) <= 0
})

if i >= len(leaves) || !bytes.Equal(leaves[i].Key, key) {
return errors.Wrap(ErrInvalidProof, "leaf key not found in proof")
}
Expand Down Expand Up @@ -207,7 +210,9 @@ func (proof *RangeProof) ComputeRootHash() []byte {
if proof == nil {
return nil
}

rootHash, _ := proof.computeRootHash()

return rootHash
}

Expand Down Expand Up @@ -283,9 +288,11 @@ func (proof *RangeProof) _computeRootHash() (rootHash []byte, treeEnd bool, err
if err != nil {
return nil, treeEnd, false, errors.Wrap(err, "recursive COMPUTEHASH call")
}

if !bytes.Equal(derivedRoot, lpath.Right) {
return nil, treeEnd, false, errors.Wrapf(ErrInvalidRoot, "intermediate root hash %X doesn't match, got %X", lpath.Right, derivedRoot)
}

if done {
return hash, treeEnd, true, nil
}
Expand Down
6 changes: 6 additions & 0 deletions proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ func TestTreeGetWithProof(t *testing.T) {
require.NoError(err)
require.NotEmpty(val)
require.NotNil(proof)

err = proof.VerifyItem(key, val)
require.Error(err, "%+v", err) // Verifying item before calling Verify(root)

err = proof.Verify(root)
require.NoError(err, "%+v", err)

err = proof.VerifyItem(key, val)
require.NoError(err, "%+v", err)

Expand All @@ -42,10 +45,13 @@ func TestTreeGetWithProof(t *testing.T) {
require.NoError(err)
require.Empty(val)
require.NotNil(proof)

err = proof.VerifyAbsence(key)
require.Error(err, "%+v", err) // Verifying absence before calling Verify(root)

err = proof.Verify(root)
require.NoError(err, "%+v", err)

err = proof.VerifyAbsence(key)
require.NoError(err, "%+v", err)
}
Expand Down