From b71d1bb5c8ab72ffac63b445ea5e60d62b648069 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 8 Nov 2022 14:31:21 +0800 Subject: [PATCH 1/7] fix: jump table is not deep copied when enable extra eips Closes: #26136 --- core/vm/interpreter.go | 6 +++--- core/vm/jump_table.go | 11 +++++++++++ core/vm/jump_table_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 core/vm/jump_table_test.go diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 312977b75588..db9ad1ef825a 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -83,14 +83,14 @@ func NewEVMInterpreter(evm *EVM, cfg Config) *EVMInterpreter { } var extraEips []int for _, eip := range cfg.ExtraEips { - copy := *cfg.JumpTable - if err := EnableEIP(eip, ©); err != nil { + copy := copyJumpTable(cfg.JumpTable) + if err := EnableEIP(eip, copy); err != nil { // Disable it, so caller can check if it's activated or not log.Error("EIP activation failed", "eip", eip, "error", err) } else { extraEips = append(extraEips, eip) } - cfg.JumpTable = © + cfg.JumpTable = copy } cfg.ExtraEips = extraEips } diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index 94229436d23c..4f0ae388890d 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -1043,3 +1043,14 @@ func newFrontierInstructionSet() JumpTable { return validate(tbl) } + +func copyJumpTable(table *JumpTable) *JumpTable { + res := *table + for i, ptr := range table { + if ptr != nil { + op := *ptr + res[i] = &op + } + } + return &res +} diff --git a/core/vm/jump_table_test.go b/core/vm/jump_table_test.go new file mode 100644 index 000000000000..8446898e0704 --- /dev/null +++ b/core/vm/jump_table_test.go @@ -0,0 +1,25 @@ +package vm + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestJumpTableCopy tests that deep copy is necessery to prevent modify shared jump table +func TestJumpTableCopy(t *testing.T) { + tbl := newMergeInstructionSet() + require.Equal(t, uint64(0), tbl[SLOAD].constantGas) + + // a deep copy won't modify the shared jump table + deepCopy := copyJumpTable(&tbl) + deepCopy[SLOAD].constantGas = 100 + require.Equal(t, uint64(100), deepCopy[SLOAD].constantGas) + require.Equal(t, uint64(0), tbl[SLOAD].constantGas) + + // but a shallow copy will modify the shared table + shallowCopy := tbl + shallowCopy[SLOAD].constantGas = 100 + require.Equal(t, uint64(100), shallowCopy[SLOAD].constantGas) + require.Equal(t, uint64(100), tbl[SLOAD].constantGas) +} From df04b01141bdf3bcab179e2c3c74f8d90025bba1 Mon Sep 17 00:00:00 2001 From: yihuang Date: Tue, 8 Nov 2022 16:40:27 +0800 Subject: [PATCH 2/7] Update core/vm/jump_table.go Co-authored-by: Martin Holst Swende --- core/vm/jump_table.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index 4f0ae388890d..7cbcc56c5adc 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -1044,13 +1044,13 @@ func newFrontierInstructionSet() JumpTable { return validate(tbl) } -func copyJumpTable(table *JumpTable) *JumpTable { - res := *table - for i, ptr := range table { - if ptr != nil { - op := *ptr - res[i] = &op +func copyJumpTable(source *JumpTable) *JumpTable { + dest := *source + for i, op := range source { + if op != nil { + opCopy := *op + dest[i] = &opCopy } } - return &res + return &dest } From 038cbdd992138a573de9762b800a3e1fd375c7a5 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 8 Nov 2022 17:04:56 +0800 Subject: [PATCH 3/7] license header --- core/vm/jump_table_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core/vm/jump_table_test.go b/core/vm/jump_table_test.go index 8446898e0704..c6cee18517a8 100644 --- a/core/vm/jump_table_test.go +++ b/core/vm/jump_table_test.go @@ -1,3 +1,19 @@ +// 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 vm import ( From 521891e60af2896f7f1e88c312462fc48ef87163 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 8 Nov 2022 17:15:32 +0800 Subject: [PATCH 4/7] less copy --- core/vm/interpreter.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index db9ad1ef825a..c99e54b76dc0 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -82,15 +82,17 @@ func NewEVMInterpreter(evm *EVM, cfg Config) *EVMInterpreter { cfg.JumpTable = &frontierInstructionSet } var extraEips []int + if len(cfg.ExtraEips) > 0 { + // Deep-copy jumptable to prevent modification of opcodes in other tables + cfg.JumpTable = copyJumpTable(cfg.JumpTable) + } for _, eip := range cfg.ExtraEips { - copy := copyJumpTable(cfg.JumpTable) - if err := EnableEIP(eip, copy); err != nil { + if err := EnableEIP(eip, cfg.JumpTable); err != nil { // Disable it, so caller can check if it's activated or not log.Error("EIP activation failed", "eip", eip, "error", err) } else { extraEips = append(extraEips, eip) } - cfg.JumpTable = copy } cfg.ExtraEips = extraEips } From 66ac0415755fffd44b01a801a1297df8d2cc16b3 Mon Sep 17 00:00:00 2001 From: yihuang Date: Tue, 8 Nov 2022 17:17:24 +0800 Subject: [PATCH 5/7] Update core/vm/jump_table_test.go Co-authored-by: Martin Holst Swende --- core/vm/jump_table_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/vm/jump_table_test.go b/core/vm/jump_table_test.go index c6cee18517a8..c972dbcbe904 100644 --- a/core/vm/jump_table_test.go +++ b/core/vm/jump_table_test.go @@ -1,4 +1,4 @@ -// Copyright 2015 The go-ethereum Authors +// Copyright 2022 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 From 157a809030b56d4df23baf60b9d8b997da979fdb Mon Sep 17 00:00:00 2001 From: yihuang Date: Tue, 8 Nov 2022 17:17:37 +0800 Subject: [PATCH 6/7] Update core/vm/jump_table_test.go Co-authored-by: Martin Holst Swende --- core/vm/jump_table_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/core/vm/jump_table_test.go b/core/vm/jump_table_test.go index c972dbcbe904..d0cae6c44024 100644 --- a/core/vm/jump_table_test.go +++ b/core/vm/jump_table_test.go @@ -33,9 +33,4 @@ func TestJumpTableCopy(t *testing.T) { require.Equal(t, uint64(100), deepCopy[SLOAD].constantGas) require.Equal(t, uint64(0), tbl[SLOAD].constantGas) - // but a shallow copy will modify the shared table - shallowCopy := tbl - shallowCopy[SLOAD].constantGas = 100 - require.Equal(t, uint64(100), shallowCopy[SLOAD].constantGas) - require.Equal(t, uint64(100), tbl[SLOAD].constantGas) } From cf029cdc0bf0270a8fee1571856c5e88ee2dd2fb Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 8 Nov 2022 14:26:35 +0100 Subject: [PATCH 7/7] Update core/vm/jump_table_test.go --- core/vm/jump_table_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/core/vm/jump_table_test.go b/core/vm/jump_table_test.go index d0cae6c44024..f67915fff3d8 100644 --- a/core/vm/jump_table_test.go +++ b/core/vm/jump_table_test.go @@ -32,5 +32,4 @@ func TestJumpTableCopy(t *testing.T) { deepCopy[SLOAD].constantGas = 100 require.Equal(t, uint64(100), deepCopy[SLOAD].constantGas) require.Equal(t, uint64(0), tbl[SLOAD].constantGas) - }