From 603822e2b9b0aa13c2699c21e2ac2bebd1a24f02 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 5 Aug 2021 10:21:35 +0300 Subject: [PATCH] native: drop Refuel method from GAS It can be used to attack the network (amplifying DOS), so it's broken beyond repair. See also neo-project/neo#2560 and neo-project/neo#2561. --- pkg/compiler/native_test.go | 4 +-- pkg/core/interop_system_test.go | 20 +---------- pkg/core/native/native_gas.go | 29 ---------------- pkg/core/native_gas_test.go | 61 --------------------------------- pkg/interop/native/gas/gas.go | 7 ---- 5 files changed, 2 insertions(+), 119 deletions(-) diff --git a/pkg/compiler/native_test.go b/pkg/compiler/native_test.go index eda0a392a1..e2a733b6ae 100644 --- a/pkg/compiler/native_test.go +++ b/pkg/compiler/native_test.go @@ -121,9 +121,7 @@ func TestNativeHelpersCompile(t *testing.T) { {"unregisterCandidate", []string{pub}}, {"getAccountState", []string{u160}}, }, nep17TestCases...)) - runNativeTestCases(t, cs.GAS.ContractMD, "gas", append([]nativeTestCase{ - {"refuel", []string{u160, "123"}}, - }, nep17TestCases...)) + runNativeTestCases(t, cs.GAS.ContractMD, "gas", nep17TestCases) runNativeTestCases(t, cs.Oracle.ContractMD, "oracle", []nativeTestCase{ {"getPrice", nil}, {"request", []string{`"url"`, "nil", `"callback"`, "nil", "123"}}, diff --git a/pkg/core/interop_system_test.go b/pkg/core/interop_system_test.go index fa5ffc4a1a..6edaca8094 100644 --- a/pkg/core/interop_system_test.go +++ b/pkg/core/interop_system_test.go @@ -683,10 +683,6 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) { emit.Opcodes(w.BinWriter, opcode.CALLT, 1, 0, opcode.RET) callT2Off := w.Len() emit.Opcodes(w.BinWriter, opcode.CALLT, 0, 0, opcode.RET) - refuelOff := w.Len() - emit.Opcodes(w.BinWriter, opcode.PUSH2, opcode.PACK) - emit.AppCallNoArgs(w.BinWriter, bc.contracts.GAS.Hash, "refuel", callflag.States|callflag.AllowNotify) - emit.Opcodes(w.BinWriter, opcode.DROP) burnGasOff := w.Len() emit.Syscall(w.BinWriter, interopnames.SystemRuntimeBurnGas) emit.Opcodes(w.BinWriter, opcode.RET) @@ -859,18 +855,8 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) { }, ReturnType: smartcontract.VoidType, }, - { - Name: "refuelGas", - Offset: refuelOff, - Parameters: []manifest.Parameter{ - manifest.NewParameter("account", smartcontract.Hash160Type), - manifest.NewParameter("gasRefuel", smartcontract.IntegerType), - manifest.NewParameter("gasBurn", smartcontract.IntegerType), - }, - ReturnType: smartcontract.VoidType, - }, } - m.Permissions = make([]manifest.Permission, 3) + m.Permissions = make([]manifest.Permission, 2) m.Permissions[0].Contract.Type = manifest.PermissionHash m.Permissions[0].Contract.Value = bc.contracts.NEO.Hash m.Permissions[0].Methods.Add("balanceOf") @@ -879,10 +865,6 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) { m.Permissions[1].Contract.Value = util.Uint160{} m.Permissions[1].Methods.Add("method") - m.Permissions[2].Contract.Type = manifest.PermissionHash - m.Permissions[2].Contract.Value = bc.contracts.GAS.Hash - m.Permissions[2].Methods.Add("refuel") - cs := &state.Contract{ ContractBase: state.ContractBase{ Hash: h, diff --git a/pkg/core/native/native_gas.go b/pkg/core/native/native_gas.go index 3f1159a773..761b14ccf3 100644 --- a/pkg/core/native/native_gas.go +++ b/pkg/core/native/native_gas.go @@ -2,20 +2,15 @@ package native import ( "errors" - "fmt" "math/big" "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/interop" - "github.com/nspcc-dev/neo-go/pkg/core/interop/runtime" "github.com/nspcc-dev/neo-go/pkg/core/native/nativenames" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/smartcontract" - "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" - "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) // GAS represents GAS native contract. @@ -45,12 +40,6 @@ func newGAS(init int64) *GAS { g.nep17TokenNative = *nep17 - desc := newDescriptor("refuel", smartcontract.VoidType, - manifest.NewParameter("account", smartcontract.Hash160Type), - manifest.NewParameter("amount", smartcontract.IntegerType)) - md := newMethodAndPrice(g.refuel, 1<<15, callflag.States|callflag.AllowNotify) - g.AddMethod(md, desc) - return g } @@ -85,24 +74,6 @@ func (g *GAS) balanceFromBytes(si *state.StorageItem) (*big.Int, error) { return &acc.Balance, err } -func (g *GAS) refuel(ic *interop.Context, args []stackitem.Item) stackitem.Item { - acc := toUint160(args[0]) - gas := toBigInt(args[1]) - - if !gas.IsInt64() || gas.Sign() == -1 { - panic("invalid GAS value") - } - - ok, err := runtime.CheckHashedWitness(ic, acc) - if !ok || err != nil { - panic(fmt.Errorf("%w: %v", ErrInvalidWitness, err)) - } - - g.burn(ic, acc, gas) - ic.VM.GasLimit += gas.Int64() - return stackitem.Null{} -} - // Initialize initializes GAS contract. func (g *GAS) Initialize(ic *interop.Context) error { if err := g.nep17TokenNative.Initialize(ic); err != nil { diff --git a/pkg/core/native_gas_test.go b/pkg/core/native_gas_test.go index 7eea4c132b..eb64c70f6b 100644 --- a/pkg/core/native_gas_test.go +++ b/pkg/core/native_gas_test.go @@ -4,74 +4,13 @@ import ( "math/big" "testing" - "github.com/nspcc-dev/neo-go/internal/random" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/require" ) -func TestGAS_Refuel(t *testing.T) { - bc := newTestChain(t) - - cs, _ := getTestContractState(bc) - require.NoError(t, bc.contracts.Management.PutContractState(bc.dao, cs)) - - const ( - sysFee = 10_000000 - burnFee = sysFee + 12345678 - ) - - accs := []*wallet.Account{ - newAccountWithGAS(t, bc), - newAccountWithGAS(t, bc), - } - - t.Run("good, refuel from self", func(t *testing.T) { - before0 := bc.GetUtilityTokenBalance(accs[0].Contract.ScriptHash()) - aer, err := invokeContractMethodGeneric(bc, sysFee, bc.contracts.GAS.Hash, "refuel", - accs[0], accs[0].Contract.ScriptHash(), int64(burnFee)) - require.NoError(t, err) - require.Equal(t, vm.HaltState, aer.VMState) - - after0 := bc.GetUtilityTokenBalance(accs[0].Contract.ScriptHash()) - tx, _, _ := bc.GetTransaction(aer.Container) - require.Equal(t, before0, new(big.Int).Add(after0, big.NewInt(tx.SystemFee+tx.NetworkFee+burnFee))) - }) - - t.Run("good, refuel from other", func(t *testing.T) { - before0 := bc.GetUtilityTokenBalance(accs[0].Contract.ScriptHash()) - before1 := bc.GetUtilityTokenBalance(accs[1].Contract.ScriptHash()) - aer, err := invokeContractMethodGeneric(bc, sysFee, cs.Hash, "refuelGas", - accs, accs[1].Contract.ScriptHash(), int64(burnFee), int64(burnFee)) - require.NoError(t, err) - require.Equal(t, vm.HaltState, aer.VMState) - - after0 := bc.GetUtilityTokenBalance(accs[0].Contract.ScriptHash()) - after1 := bc.GetUtilityTokenBalance(accs[1].Contract.ScriptHash()) - - tx, _, _ := bc.GetTransaction(aer.Container) - require.Equal(t, before0, new(big.Int).Add(after0, big.NewInt(tx.SystemFee+tx.NetworkFee))) - require.Equal(t, before1, new(big.Int).Add(after1, big.NewInt(burnFee))) - }) - - t.Run("bad, invalid witness", func(t *testing.T) { - aer, err := invokeContractMethodGeneric(bc, sysFee, cs.Hash, "refuelGas", - accs, random.Uint160(), int64(1), int64(1)) - require.NoError(t, err) - require.Equal(t, vm.FaultState, aer.VMState) - }) - - t.Run("bad, invalid GAS amount", func(t *testing.T) { - aer, err := invokeContractMethodGeneric(bc, sysFee, cs.Hash, "refuelGas", - accs, accs[0].Contract.ScriptHash(), int64(0), int64(1)) - require.NoError(t, err) - require.Equal(t, vm.FaultState, aer.VMState) - }) -} - func TestGAS_Roundtrip(t *testing.T) { bc := newTestChain(t) diff --git a/pkg/interop/native/gas/gas.go b/pkg/interop/native/gas/gas.go index fbecafd126..1d0cc9329b 100644 --- a/pkg/interop/native/gas/gas.go +++ b/pkg/interop/native/gas/gas.go @@ -37,10 +37,3 @@ func Transfer(from, to interop.Hash160, amount int, data interface{}) bool { return contract.Call(interop.Hash160(Hash), "transfer", contract.All, from, to, amount, data).(bool) } - -// Refuel makes some GAS from the provided account available -// for the current execution. It represents `refuel` method of GAS native contract. -func Refuel(from interop.Hash160, amount int) { - contract.Call(interop.Hash160(Hash), "refuel", - contract.States|contract.AllowNotify, from, amount) -}