Sync geth: security fix

cmd/geth/main.go

@@ -193,6 +193,7 @@ var (
 		utils.RPCGlobalGasCapFlag,
 		utils.RPCGlobalEVMTimeoutFlag,
 		utils.RPCGlobalTxFeeCapFlag,
+		utils.RPCGlobalLogQueryLimit,
 		utils.AllowUnprotectedTxs,
 		utils.MaxBlockRangeFlag,
 		utils.BatchRequestLimit,

cmd/geth/usage.go

@@ -160,6 +160,7 @@ var AppHelpFlagGroups = []flags.FlagGroup{
 			utils.RPCGlobalGasCapFlag,
 			utils.RPCGlobalEVMTimeoutFlag,
 			utils.RPCGlobalTxFeeCapFlag,
+			utils.RPCGlobalLogQueryLimit,
 			utils.AllowUnprotectedTxs,
 			utils.JSpathFlag,
 			utils.ExecFlag,

cmd/utils/flags.go

@@ -560,6 +560,11 @@ var (
 		Usage: "Sets a cap on transaction fee (in ether) that can be sent via the RPC APIs (0 = no cap)",
 		Value: ethconfig.Defaults.RPCTxFeeCap,
 	}
+	RPCGlobalLogQueryLimit = &cli.IntFlag{
+		Name:  "rpc.logquerylimit",
+		Usage: "Maximum number of alternative addresses or topics allowed per search position in eth_getLogs filter criteria (0 = no cap)",
+		Value: ethconfig.Defaults.LogQueryLimit,
+	}
 	// Logging and debug settings
 	EthStatsURLFlag = cli.StringFlag{
 		Name:  "ethstats",
@@ -1707,6 +1712,9 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
 	if ctx.IsSet(CacheLogSizeFlag.Name) {
 		cfg.FilterLogCacheSize = ctx.Int(CacheLogSizeFlag.Name)
 	}
+	if ctx.IsSet(RPCGlobalLogQueryLimit.Name) {
+		cfg.LogQueryLimit = ctx.Int(RPCGlobalLogQueryLimit.Name)
+	}
 	if !ctx.Bool(SnapshotFlag.Name) {
 		// If snap-sync is requested, this flag is also required
 		if cfg.SyncMode == downloader.SnapSync {
@@ -1982,7 +1990,8 @@ func RegisterGraphQLService(stack *node.Node, backend ethapi.Backend, filterSyst
 func RegisterFilterAPI(stack *node.Node, backend ethapi.Backend, ethcfg *ethconfig.Config) *filters.FilterSystem {
 	isLightClient := ethcfg.SyncMode == downloader.LightSync
 	filterSystem := filters.NewFilterSystem(backend, filters.Config{
-		LogCacheSize: ethcfg.FilterLogCacheSize,
+		LogCacheSize:  ethcfg.FilterLogCacheSize,
+		LogQueryLimit: ethcfg.LogQueryLimit,
 	})
 	stack.RegisterAPIs([]rpc.API{{
 		Namespace: "eth",

core/state/statedb_hooked.go

@@ -174,7 +174,10 @@ func (s *hookedStateDB) SetCode(address common.Address, code []byte) []byte {
 		if len(prev) != 0 {
 			prevHash = crypto.Keccak256Hash(prev)
 		}
-		s.hooks.OnCodeChange(address, prevHash, prev, crypto.Keccak256Hash(code), code)
+		newHash := crypto.Keccak256Hash(code)
+		if prevHash != newHash {
+			s.hooks.OnCodeChange(address, prevHash, prev, newHash, code)
+		}
 	}
 	return prev
 }

core/state/statedb_hooked_test.go

@@ -0,0 +1,41 @@
+package state
+
+import (
+	"testing"
+
+	"github.com/morph-l2/go-ethereum/common"
+	"github.com/morph-l2/go-ethereum/core/rawdb"
+	"github.com/morph-l2/go-ethereum/core/tracing"
+)
+
+func TestSetCodeNoChangeDoesNotHook(t *testing.T) {
+	statedb, err := New(common.Hash{}, NewDatabase(rawdb.NewMemoryDatabase()), nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	addr := common.HexToAddress("0x1111111111111111111111111111111111111111")
+	var calls int
+	hooked := NewHookedState(statedb, &tracing.Hooks{
+		OnCodeChange: func(common.Address, common.Hash, []byte, common.Hash, []byte) {
+			calls++
+		},
+	})
+
+	code := []byte{1, 2, 3}
+	hooked.SetCode(addr, code)
+	if calls != 1 {
+		t.Fatalf("first SetCode should hook once, got %d", calls)
+	}
+	hooked.SetCode(addr, append([]byte(nil), code...))
+	if calls != 1 {
+		t.Fatalf("unchanged SetCode should not hook, got %d", calls)
+	}
+	hooked.SetCode(addr, nil)
+	if calls != 2 {
+		t.Fatalf("clearing code should hook once, got %d", calls)
+	}
+	hooked.SetCode(addr, nil)
+	if calls != 2 {
+		t.Fatalf("repeated empty SetCode should not hook, got %d", calls)
+	}
+}

core/vm/evm.go

@@ -500,7 +500,9 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
 	if err == nil {
 		createDataGas := uint64(len(ret)) * params.CreateDataGas
 		if contract.UseGas(createDataGas, evm.Config.Tracer, tracing.GasChangeCallCodeStorage) {
-			evm.StateDB.SetCode(address, ret)
+			if len(ret) > 0 {
+				evm.StateDB.SetCode(address, ret)
+			}
 		} else {
 			err = ErrCodeStoreOutOfGas
 		}

core/vm/evm_create_test.go

@@ -0,0 +1,86 @@
+package vm
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/morph-l2/go-ethereum/common"
+	"github.com/morph-l2/go-ethereum/core/rawdb"
+	statedb "github.com/morph-l2/go-ethereum/core/state"
+	"github.com/morph-l2/go-ethereum/params"
+)
+
+type setCodeCountingStateDB struct {
+	StateDB
+	setCodeCalls int
+}
+
+func (db *setCodeCountingStateDB) SetCode(addr common.Address, code []byte) []byte {
+	db.setCodeCalls++
+	return db.StateDB.SetCode(addr, code)
+}
+
+func newCreateTestEVM(t *testing.T) (*EVM, *setCodeCountingStateDB, common.Address) {
+	t.Helper()
+
+	base, err := statedb.New(common.Hash{}, statedb.NewDatabase(rawdb.NewMemoryDatabase()), nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	caller := common.HexToAddress("0x1111111111111111111111111111111111111111")
+	base.CreateAccount(caller)
+
+	wrapped := &setCodeCountingStateDB{StateDB: base}
+	blockCtx := BlockContext{
+		CanTransfer: func(StateDB, common.Address, *big.Int) bool { return true },
+		Transfer:    func(StateDB, common.Address, common.Address, *big.Int) {},
+		GetHash:     func(uint64) common.Hash { return common.Hash{} },
+		BlockNumber: big.NewInt(0),
+		Time:        big.NewInt(0),
+		GasLimit:    10_000_000,
+		BaseFee:     big.NewInt(params.InitialBaseFee),
+	}
+	txCtx := TxContext{
+		Origin:   caller,
+		GasPrice: big.NewInt(params.InitialBaseFee),
+	}
+	return NewEVM(blockCtx, txCtx, wrapped, params.TestChainConfig, Config{}), wrapped, caller
+}
+
+func TestCreateEmptyReturnSkipsSetCode(t *testing.T) {
+	evm, statedb, caller := newCreateTestEVM(t)
+
+	ret, _, _, err := evm.Create(AccountRef(caller), nil, 1_000_000, new(big.Int))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(ret) != 0 {
+		t.Fatalf("expected empty create return, got %x", ret)
+	}
+	if statedb.setCodeCalls != 0 {
+		t.Fatalf("empty create called SetCode %d times", statedb.setCodeCalls)
+	}
+}
+
+func TestCreateNonEmptyReturnSetsCode(t *testing.T) {
+	evm, statedb, caller := newCreateTestEVM(t)
+
+	code := []byte{
+		byte(PUSH1), 0x2a,
+		byte(PUSH1), 0x00,
+		byte(MSTORE),
+		byte(PUSH1), 0x20,
+		byte(PUSH1), 0x00,
+		byte(RETURN),
+	}
+	ret, _, _, err := evm.Create(AccountRef(caller), code, 1_000_000, new(big.Int))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(ret) != 32 {
+		t.Fatalf("expected non-empty create return, got %x", ret)
+	}
+	if statedb.setCodeCalls != 1 {
+		t.Fatalf("non-empty create called SetCode %d times", statedb.setCodeCalls)
+	}
+}

eth/ethconfig/config.go

@@ -83,6 +83,7 @@ var Defaults = Config{
 	TrieTimeout:             60 * time.Minute,
 	SnapshotCache:           102,
 	FilterLogCacheSize:      32,
+	LogQueryLimit:           1000,
 	Miner:                   miner.DefaultConfig,
 	TxPool:                  core.DefaultTxPoolConfig,
 	RPCGasCap:               50000000,
@@ -169,6 +170,9 @@ type Config struct {
 	// This is the number of blocks for which logs will be cached in the filter system.
 	FilterLogCacheSize int
 
+	// This is the maximum number of addresses or topics allowed in filter criteria.
+	LogQueryLimit int
+
 	// Mining options
 	Miner miner.Config
 

eth/fetcher/tx_fetcher.go

@@ -674,6 +674,10 @@ func (f *TxFetcher) loop() {
 					if len(f.announced[hash]) == 0 {
 						delete(f.announced, hash)
 					}
+					delete(f.alternates[hash], drop.peer)
+					if len(f.alternates[hash]) == 0 {
+						delete(f.alternates, hash)
+					}
 				}
 				delete(f.announces, drop.peer)
 			}

eth/fetcher/tx_fetcher_test.go

@@ -1121,6 +1121,44 @@ func TestTransactionFetcherDropRescheduling(t *testing.T) {
 	})
 }
 
+func TestTransactionFetcherDropAlternates(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "B", hashes: []common.Hash{{0x01}}},
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}},
+					"B": {{0x01}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}},
+				},
+			},
+			doDrop("B"),
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}},
+				},
+			},
+		},
+	})
+}
+
 // This test reproduces a crash caught by the fuzzer. The root cause was a
 // dangling transaction timing out and clashing on readd with a concurrently
 // announced one.