From 886478b18b73bbe8421531f1a71664a2bc0f5eeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 20 Jan 2016 12:09:24 +0200 Subject: [PATCH 1/2] core/state, trie: don't leak database writes before commit --- core/state/statedb.go | 10 +++++--- core/state/statedb_test.go | 52 ++++++++++++++++++++++++++++++++++++++ trie/secure_trie.go | 49 +++++++++++++++++++++++++++++++---- 3 files changed, 103 insertions(+), 8 deletions(-) create mode 100644 core/state/statedb_test.go diff --git a/core/state/statedb.go b/core/state/statedb.go index 8093472b5..22ffa36a0 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -206,9 +206,6 @@ func (self *StateDB) Delete(addr common.Address) bool { // Update the given state object and apply it to state trie func (self *StateDB) UpdateStateObject(stateObject *StateObject) { - if len(stateObject.code) > 0 { - self.db.Put(stateObject.codeHash, stateObject.code) - } addr := stateObject.Address() data, err := rlp.EncodeToBytes(stateObject) if err != nil { @@ -375,8 +372,15 @@ func (s *StateDB) commit(db trie.DatabaseWriter) (common.Hash, error) { // and just mark it for deletion in the trie. s.DeleteStateObject(stateObject) } else { + // Write any contract code associated with the state object + if len(stateObject.code) > 0 { + if err := db.Put(stateObject.codeHash, stateObject.code); err != nil { + return common.Hash{}, err + } + } // Write any storage changes in the state object to its trie. stateObject.Update() + // Commit the trie of the object to the batch. // This updates the trie root internally, so // getting the root hash of the storage trie diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go new file mode 100644 index 000000000..fd4d02fdd --- /dev/null +++ b/core/state/statedb_test.go @@ -0,0 +1,52 @@ +// Copyright 2015 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package state + +import ( + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethdb" +) + +// Tests that updating a state trie does not leak any database writes prior to +// actually committing the state. +func TestUpdateLeaks(t *testing.T) { + // Create an empty state database + db, _ := ethdb.NewMemDatabase() + state, _ := New(common.Hash{}, db) + + // Update it with some accounts + for i := byte(0); i < 255; i++ { + obj := state.GetOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.AddBalance(big.NewInt(int64(11 * i))) + obj.SetNonce(uint64(42 * i)) + if i%2 == 0 { + obj.SetState(common.BytesToHash([]byte{i, i, i}), common.BytesToHash([]byte{i, i, i, i})) + } + if i%3 == 0 { + obj.SetCode([]byte{i, i, i, i, i}) + } + state.UpdateStateObject(obj) + } + // Ensure that no data was leaked into the database + for _, key := range db.Keys() { + value, _ := db.Get(key) + t.Errorf("State leaked into database: %x -> %x", key, value) + } +} diff --git a/trie/secure_trie.go b/trie/secure_trie.go index caeef3c3a..ca515aacb 100644 --- a/trie/secure_trie.go +++ b/trie/secure_trie.go @@ -40,9 +40,10 @@ var secureKeyPrefix = []byte("secure-key-") type SecureTrie struct { *Trie - hash hash.Hash - secKeyBuf []byte - hashKeyBuf []byte + hash hash.Hash + hashKeyBuf []byte + secKeyBuf []byte + secKeyCache map[string][]byte } // NewSecure creates a trie with an existing root node from db. @@ -59,7 +60,10 @@ func NewSecure(root common.Hash, db Database) (*SecureTrie, error) { if err != nil { return nil, err } - return &SecureTrie{Trie: trie}, nil + return &SecureTrie{ + Trie: trie, + secKeyCache: make(map[string][]byte), + }, nil } // Get returns the value for key stored in the trie. @@ -105,7 +109,7 @@ func (t *SecureTrie) TryUpdate(key, value []byte) error { if err != nil { return err } - t.Trie.db.Put(t.secKey(hk), key) + t.secKeyCache[string(hk)] = key return nil } @@ -125,10 +129,45 @@ func (t *SecureTrie) TryDelete(key []byte) error { // GetKey returns the sha3 preimage of a hashed key that was // previously used to store a value. func (t *SecureTrie) GetKey(shaKey []byte) []byte { + if key, ok := t.secKeyCache[string(shaKey)]; ok { + return key + } key, _ := t.Trie.db.Get(t.secKey(shaKey)) return key } +// Commit writes all nodes and the secure hash pre-images to the trie's database. +// Nodes are stored with their sha3 hash as the key. +// +// Committing flushes nodes from memory. Subsequent Get calls will load nodes +// from the database. +func (t *SecureTrie) Commit() (root common.Hash, err error) { + return t.CommitTo(t.db) +} + +// CommitTo writes all nodes and the secure hash pre-images to the given database. +// Nodes are stored with their sha3 hash as the key. +// +// Committing flushes nodes from memory. Subsequent Get calls will load nodes from +// the trie's database. Calling code must ensure that the changes made to db are +// written back to the trie's attached database before using the trie. +func (t *SecureTrie) CommitTo(db DatabaseWriter) (root common.Hash, err error) { + if len(t.secKeyCache) > 0 { + for hk, key := range t.secKeyCache { + if err := db.Put(t.secKey([]byte(hk)), key); err != nil { + return common.Hash{}, err + } + } + t.secKeyCache = make(map[string][]byte) + } + n, err := t.hashRoot(db) + if err != nil { + return (common.Hash{}), err + } + t.root = n + return common.BytesToHash(n.(hashNode)), nil +} + func (t *SecureTrie) secKey(key []byte) []byte { t.secKeyBuf = append(t.secKeyBuf[:0], secureKeyPrefix...) t.secKeyBuf = append(t.secKeyBuf, key...) From f3d4ce0d164f7b17a143304e2b94421573d596a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 20 Jan 2016 16:06:28 +0200 Subject: [PATCH 2/2] core/state, ethdb, trie: test intermediate secure key leak, fix memdb bug --- core/state/statedb_test.go | 68 ++++++++++++++++++++++++++++++++++++++ ethdb/memory_database.go | 2 +- trie/secure_trie.go | 6 ++-- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index fd4d02fdd..8138f8d78 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -50,3 +50,71 @@ func TestUpdateLeaks(t *testing.T) { t.Errorf("State leaked into database: %x -> %x", key, value) } } + +// Tests that no intermediate state of an object is stored into the database, +// only the one right before the commit. +func TestIntermediateLeaks(t *testing.T) { + // Create two state databases, one transitioning to the final state, the other final from the beginning + transDb, _ := ethdb.NewMemDatabase() + finalDb, _ := ethdb.NewMemDatabase() + transState, _ := New(common.Hash{}, transDb) + finalState, _ := New(common.Hash{}, finalDb) + + // Update the states with some objects + for i := byte(0); i < 255; i++ { + // Create a new state object with some data into the transition database + obj := transState.GetOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.SetBalance(big.NewInt(int64(11 * i))) + obj.SetNonce(uint64(42 * i)) + if i%2 == 0 { + obj.SetState(common.BytesToHash([]byte{i, i, i, 0}), common.BytesToHash([]byte{i, i, i, i, 0})) + } + if i%3 == 0 { + obj.SetCode([]byte{i, i, i, i, i, 0}) + } + transState.UpdateStateObject(obj) + + // Overwrite all the data with new values in the transition database + obj.SetBalance(big.NewInt(int64(11*i + 1))) + obj.SetNonce(uint64(42*i + 1)) + if i%2 == 0 { + obj.SetState(common.BytesToHash([]byte{i, i, i, 0}), common.Hash{}) + obj.SetState(common.BytesToHash([]byte{i, i, i, 1}), common.BytesToHash([]byte{i, i, i, i, 1})) + } + if i%3 == 0 { + obj.SetCode([]byte{i, i, i, i, i, 1}) + } + transState.UpdateStateObject(obj) + + // Create the final state object directly in the final database + obj = finalState.GetOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.SetBalance(big.NewInt(int64(11*i + 1))) + obj.SetNonce(uint64(42*i + 1)) + if i%2 == 0 { + obj.SetState(common.BytesToHash([]byte{i, i, i, 1}), common.BytesToHash([]byte{i, i, i, i, 1})) + } + if i%3 == 0 { + obj.SetCode([]byte{i, i, i, i, i, 1}) + } + finalState.UpdateStateObject(obj) + } + if _, err := transState.Commit(); err != nil { + t.Fatalf("failed to commit transition state: %v", err) + } + if _, err := finalState.Commit(); err != nil { + t.Fatalf("failed to commit final state: %v", err) + } + // Cross check the databases to ensure they are the same + for _, key := range finalDb.Keys() { + if _, err := transDb.Get(key); err != nil { + val, _ := finalDb.Get(key) + t.Errorf("entry missing from the transition database: %x -> %x", key, val) + } + } + for _, key := range transDb.Keys() { + if _, err := finalDb.Get(key); err != nil { + val, _ := transDb.Get(key) + t.Errorf("extra entry in the transition database: %x -> %x", key, val) + } + } +} diff --git a/ethdb/memory_database.go b/ethdb/memory_database.go index 45423ed73..a729f5233 100644 --- a/ethdb/memory_database.go +++ b/ethdb/memory_database.go @@ -107,7 +107,7 @@ func (b *memBatch) Put(key, value []byte) error { b.lock.Lock() defer b.lock.Unlock() - b.writes = append(b.writes, kv{key, common.CopyBytes(value)}) + b.writes = append(b.writes, kv{common.CopyBytes(key), common.CopyBytes(value)}) return nil } diff --git a/trie/secure_trie.go b/trie/secure_trie.go index ca515aacb..be7defe83 100644 --- a/trie/secure_trie.go +++ b/trie/secure_trie.go @@ -109,7 +109,7 @@ func (t *SecureTrie) TryUpdate(key, value []byte) error { if err != nil { return err } - t.secKeyCache[string(hk)] = key + t.secKeyCache[string(hk)] = common.CopyBytes(key) return nil } @@ -123,7 +123,9 @@ func (t *SecureTrie) Delete(key []byte) { // TryDelete removes any existing value for key from the trie. // If a node was not found in the database, a MissingNodeError is returned. func (t *SecureTrie) TryDelete(key []byte) error { - return t.Trie.TryDelete(t.hashKey(key)) + hk := t.hashKey(key) + delete(t.secKeyCache, string(hk)) + return t.Trie.TryDelete(hk) } // GetKey returns the sha3 preimage of a hashed key that was