From da2db74bc91ea9af42e9897bc99e2965ebf67746 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 22 Jul 2022 11:44:11 +0300 Subject: [PATCH 1/3] contracts: decipher oracle test contract Make it a proper Go contract, opcodes are fun until you want to change something in them. Part of #2412. --- internal/contracts/contracts.go | 6 +- internal/contracts/contracts_test.go | 77 +----------------- internal/contracts/oracle_contract/go.mod | 5 ++ internal/contracts/oracle_contract/go.sum | 2 + internal/contracts/oracle_contract/oracle.go | 24 ++++++ .../oracle_contract/oracle.manifest.json | 2 +- internal/contracts/oracle_contract/oracle.nef | Bin 199 -> 233 bytes internal/contracts/oracle_contract/oracle.yml | 6 ++ 8 files changed, 45 insertions(+), 77 deletions(-) create mode 100644 internal/contracts/oracle_contract/go.mod create mode 100644 internal/contracts/oracle_contract/go.sum create mode 100644 internal/contracts/oracle_contract/oracle.go create mode 100644 internal/contracts/oracle_contract/oracle.yml diff --git a/internal/contracts/contracts.go b/internal/contracts/contracts.go index dc381d5c6..210edd0b6 100644 --- a/internal/contracts/contracts.go +++ b/internal/contracts/contracts.go @@ -21,8 +21,10 @@ var ( helper2ContractNEFPath = filepath.Join("management_helper", "management_helper2.nef") helper2ContractManifestPath = filepath.Join("management_helper", "management_helper2.manifest.json") - oracleContractNEFPath = filepath.Join("oracle_contract", "oracle.nef") - oracleContractManifestPath = filepath.Join("oracle_contract", "oracle.manifest.json") + oracleContractModPath = "oracle_contract" + oracleContractYAMLPath = filepath.Join(oracleContractModPath, "oracle.yml") + oracleContractNEFPath = filepath.Join(oracleContractModPath, "oracle.nef") + oracleContractManifestPath = filepath.Join(oracleContractModPath, "oracle.manifest.json") ) // GetTestContractState reads 2 pre-compiled contracts generated by diff --git a/internal/contracts/contracts_test.go b/internal/contracts/contracts_test.go index df9b4e80f..e9e027b36 100644 --- a/internal/contracts/contracts_test.go +++ b/internal/contracts/contracts_test.go @@ -41,81 +41,10 @@ func TestGenerateHelperContracts(t *testing.T) { // Oracle and StdLib native hashes and saves the generated NEF and manifest to `oracle_contract` folder. // Set `saveState` flag to true and run the test to rewrite NEF and manifest files. func generateOracleContract(t *testing.T, saveState bool) { - bc, validator, committee := chain.NewMultiWithCustomConfig(t, func(c *config.ProtocolConfiguration) { - c.P2PSigExtensions = true - }) - e := neotest.NewExecutor(t, bc, validator, committee) - - oracleHash := e.NativeHash(t, nativenames.Oracle) - stdHash := e.NativeHash(t, nativenames.StdLib) - - w := io.NewBufBinWriter() - emit.Int(w.BinWriter, 5) - emit.Opcodes(w.BinWriter, opcode.PACK) - emit.Int(w.BinWriter, int64(callflag.All)) - emit.String(w.BinWriter, "request") - emit.Bytes(w.BinWriter, oracleHash.BytesBE()) - emit.Syscall(w.BinWriter, interopnames.SystemContractCall) - emit.Opcodes(w.BinWriter, opcode.DROP) - emit.Opcodes(w.BinWriter, opcode.RET) - - // `handle` method aborts if len(userData) == 2 and does NOT perform witness checks - // for the sake of contract code simplicity (the contract is used in multiple testchains). - offset := w.Len() - - emit.Opcodes(w.BinWriter, opcode.OVER) - emit.Opcodes(w.BinWriter, opcode.SIZE) - emit.Int(w.BinWriter, 2) - emit.Instruction(w.BinWriter, opcode.JMPNE, []byte{3}) - emit.Opcodes(w.BinWriter, opcode.ABORT) - emit.Int(w.BinWriter, 4) // url, userData, code, result - emit.Opcodes(w.BinWriter, opcode.PACK) - emit.Int(w.BinWriter, 1) // 1 byte (args count for `serialize`) - emit.Opcodes(w.BinWriter, opcode.PACK) // 1 byte (pack args into array for `serialize`) - emit.AppCallNoArgs(w.BinWriter, stdHash, "serialize", callflag.All) // 39 bytes - emit.String(w.BinWriter, "lastOracleResponse") - emit.Syscall(w.BinWriter, interopnames.SystemStorageGetContext) - emit.Syscall(w.BinWriter, interopnames.SystemStoragePut) - emit.Opcodes(w.BinWriter, opcode.RET) - - m := manifest.NewManifest("TestOracle") - m.ABI.Methods = []manifest.Method{ - { - Name: "requestURL", - Offset: 0, - Parameters: []manifest.Parameter{ - manifest.NewParameter("url", smartcontract.StringType), - manifest.NewParameter("filter", smartcontract.StringType), - manifest.NewParameter("callback", smartcontract.StringType), - manifest.NewParameter("userData", smartcontract.AnyType), - manifest.NewParameter("gasForResponse", smartcontract.IntegerType), - }, - ReturnType: smartcontract.VoidType, - }, - { - Name: "handle", - Offset: offset, - Parameters: []manifest.Parameter{ - manifest.NewParameter("url", smartcontract.StringType), - manifest.NewParameter("userData", smartcontract.AnyType), - manifest.NewParameter("code", smartcontract.IntegerType), - manifest.NewParameter("result", smartcontract.ByteArrayType), - }, - ReturnType: smartcontract.VoidType, - }, - } - - perm := manifest.NewPermission(manifest.PermissionHash, oracleHash) - perm.Methods.Add("request") - m.Permissions = append(m.Permissions, *perm) - - // Generate NEF file. - script := w.Bytes() - ne, err := nef.NewFile(script) - require.NoError(t, err) + ctr := neotest.CompileFile(t, util.Uint160{}, oracleContractModPath, oracleContractYAMLPath) // Write NEF file. - bytes, err := ne.Bytes() + bytes, err := ctr.NEF.Bytes() require.NoError(t, err) if saveState { err = os.WriteFile(oracleContractNEFPath, bytes, os.ModePerm) @@ -123,7 +52,7 @@ func generateOracleContract(t *testing.T, saveState bool) { } // Write manifest file. - mData, err := json.Marshal(m) + mData, err := json.Marshal(ctr.Manifest) require.NoError(t, err) if saveState { err = os.WriteFile(oracleContractManifestPath, mData, os.ModePerm) diff --git a/internal/contracts/oracle_contract/go.mod b/internal/contracts/oracle_contract/go.mod new file mode 100644 index 000000000..6429852ea --- /dev/null +++ b/internal/contracts/oracle_contract/go.mod @@ -0,0 +1,5 @@ +module github.com/nspcc-dev/neo-go/examples/oracle + +go 1.17 + +require github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20220809123759-3094d3e0c14b diff --git a/internal/contracts/oracle_contract/go.sum b/internal/contracts/oracle_contract/go.sum new file mode 100644 index 000000000..91379fe47 --- /dev/null +++ b/internal/contracts/oracle_contract/go.sum @@ -0,0 +1,2 @@ +github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20220809123759-3094d3e0c14b h1:J7QZNmnO84esVuPbBo88fwAG4XVnDjlSTiO1ewLNCkQ= +github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20220809123759-3094d3e0c14b/go.mod h1:23bBw0v6pBYcrWs8CBEEDIEDJNbcFoIh8pGGcf2Vv8s= diff --git a/internal/contracts/oracle_contract/oracle.go b/internal/contracts/oracle_contract/oracle.go new file mode 100644 index 000000000..ba81c4005 --- /dev/null +++ b/internal/contracts/oracle_contract/oracle.go @@ -0,0 +1,24 @@ +package oraclecontract + +import ( + "github.com/nspcc-dev/neo-go/pkg/interop/native/oracle" + "github.com/nspcc-dev/neo-go/pkg/interop/native/std" + "github.com/nspcc-dev/neo-go/pkg/interop/storage" + "github.com/nspcc-dev/neo-go/pkg/interop/util" +) + +// RequestURL accepts a complete set of parameters to make an oracle request and +// performs it. +func RequestURL(url string, filter []byte, callback string, userData interface{}, gasForResponse int) { + oracle.Request(url, filter, callback, userData, gasForResponse) +} + +// Handle is a response handler that writes response data to the storage. +func Handle(url string, data interface{}, code int, res []byte) { + // ABORT if len(data) == 2, some tests use this feature. + if data != nil && len(data.(string)) == 2 { + util.Abort() + } + params := []interface{}{url, data, code, res} + storage.Put(storage.GetContext(), "lastOracleResponse", std.Serialize(params)) +} diff --git a/internal/contracts/oracle_contract/oracle.manifest.json b/internal/contracts/oracle_contract/oracle.manifest.json index 85ed7ffc0..36fb0f38b 100644 --- a/internal/contracts/oracle_contract/oracle.manifest.json +++ b/internal/contracts/oracle_contract/oracle.manifest.json @@ -1 +1 @@ -{"name":"TestOracle","abi":{"methods":[{"name":"requestURL","offset":0,"parameters":[{"name":"url","type":"String"},{"name":"filter","type":"String"},{"name":"callback","type":"String"},{"name":"userData","type":"Any"},{"name":"gasForResponse","type":"Integer"}],"returntype":"Void","safe":false},{"name":"handle","offset":41,"parameters":[{"name":"url","type":"String"},{"name":"userData","type":"Any"},{"name":"code","type":"Integer"},{"name":"result","type":"ByteArray"}],"returntype":"Void","safe":false}],"events":[]},"features":{},"groups":[],"permissions":[{"contract":"0xfe924b7cfe89ddd271abaf7210a80a7e11178758","methods":["request"]}],"supportedstandards":[],"trusts":[],"extra":null} \ No newline at end of file +{"name":"Oracle test","abi":{"methods":[{"name":"handle","offset":14,"parameters":[{"name":"url","type":"String"},{"name":"data","type":"Any"},{"name":"code","type":"Integer"},{"name":"res","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"requestURL","offset":0,"parameters":[{"name":"url","type":"String"},{"name":"filter","type":"ByteArray"},{"name":"callback","type":"String"},{"name":"userData","type":"Any"},{"name":"gasForResponse","type":"Integer"}],"returntype":"Void","safe":false}],"events":[]},"features":{},"groups":[],"permissions":[{"contract":"*","methods":["request"]}],"supportedstandards":[],"trusts":[],"extra":null} \ No newline at end of file diff --git a/internal/contracts/oracle_contract/oracle.nef b/internal/contracts/oracle_contract/oracle.nef index a09f68cc79a985602a7eac92993263e49153cb9f..c7f91865fa387344ca8f5d2617b228ec9fc8a553 100644 GIT binary patch literal 233 zcmeZsbu-RO&DTxO*9B4~sl_D>WB{g!c5%Tvt`!1B>sJ?Ey4(4$#(UB~_M+6nQlNpX z3=G@{-dmn~@Z_cHu|o^8Dj&M=-pD_7+OsIZ<=VsRA^(qgu#uBxmMIZ)s@`&;_C!VGgphN4U! kp`66x691ya_n5-ko>YIht1097|d2LJ#7 literal 199 zcmeZsbu-RO&DTxO*JYpxREi#u=V32OEi6qfF5wZ0Xcrf(<60q5w0?ErrMsQ~YP=`? zb4;p@4svzyJ|(2ZY$0+$5U7E(IJGD Date: Fri, 22 Jul 2022 12:14:54 +0300 Subject: [PATCH 2/3] native: add Oracle.finish reentrancy test --- internal/contracts/oracle_contract/oracle.go | 18 ++++++++++++++++++ .../oracle_contract/oracle.manifest.json | 2 +- internal/contracts/oracle_contract/oracle.nef | Bin 233 -> 467 bytes internal/contracts/oracle_contract/oracle.yml | 3 ++- pkg/core/native/native_test/oracle_test.go | 10 ++++++++++ 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/internal/contracts/oracle_contract/oracle.go b/internal/contracts/oracle_contract/oracle.go index ba81c4005..da7ebe990 100644 --- a/internal/contracts/oracle_contract/oracle.go +++ b/internal/contracts/oracle_contract/oracle.go @@ -1,8 +1,11 @@ package oraclecontract import ( + "github.com/nspcc-dev/neo-go/pkg/interop" + "github.com/nspcc-dev/neo-go/pkg/interop/contract" "github.com/nspcc-dev/neo-go/pkg/interop/native/oracle" "github.com/nspcc-dev/neo-go/pkg/interop/native/std" + "github.com/nspcc-dev/neo-go/pkg/interop/runtime" "github.com/nspcc-dev/neo-go/pkg/interop/storage" "github.com/nspcc-dev/neo-go/pkg/interop/util" ) @@ -22,3 +25,18 @@ func Handle(url string, data interface{}, code int, res []byte) { params := []interface{}{url, data, code, res} storage.Put(storage.GetContext(), "lastOracleResponse", std.Serialize(params)) } + +// HandleRecursive invokes oracle.finish again to test Oracle reentrance. +func HandleRecursive(url string, data interface{}, code int, res []byte) { + // Regular safety check. + callingHash := runtime.GetCallingScriptHash() + if !callingHash.Equals(oracle.Hash) { + panic("not called from oracle contract") + } + + runtime.Notify("Invocation") + if runtime.GetInvocationCounter() == 1 { + // We provide no wrapper for finish in interops, it's not usually needed. + contract.Call(interop.Hash160(oracle.Hash), "finish", contract.All) + } +} diff --git a/internal/contracts/oracle_contract/oracle.manifest.json b/internal/contracts/oracle_contract/oracle.manifest.json index 36fb0f38b..e4c102428 100644 --- a/internal/contracts/oracle_contract/oracle.manifest.json +++ b/internal/contracts/oracle_contract/oracle.manifest.json @@ -1 +1 @@ -{"name":"Oracle test","abi":{"methods":[{"name":"handle","offset":14,"parameters":[{"name":"url","type":"String"},{"name":"data","type":"Any"},{"name":"code","type":"Integer"},{"name":"res","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"requestURL","offset":0,"parameters":[{"name":"url","type":"String"},{"name":"filter","type":"ByteArray"},{"name":"callback","type":"String"},{"name":"userData","type":"Any"},{"name":"gasForResponse","type":"Integer"}],"returntype":"Void","safe":false}],"events":[]},"features":{},"groups":[],"permissions":[{"contract":"*","methods":["request"]}],"supportedstandards":[],"trusts":[],"extra":null} \ No newline at end of file +{"name":"Oracle test","abi":{"methods":[{"name":"handle","offset":14,"parameters":[{"name":"url","type":"String"},{"name":"data","type":"Any"},{"name":"code","type":"Integer"},{"name":"res","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"handleRecursive","offset":89,"parameters":[{"name":"url","type":"String"},{"name":"data","type":"Any"},{"name":"code","type":"Integer"},{"name":"res","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"requestURL","offset":0,"parameters":[{"name":"url","type":"String"},{"name":"filter","type":"ByteArray"},{"name":"callback","type":"String"},{"name":"userData","type":"Any"},{"name":"gasForResponse","type":"Integer"}],"returntype":"Void","safe":false}],"events":[{"name":"Invocation","parameters":null}]},"features":{},"groups":[],"permissions":[{"contract":"*","methods":["request","finish"]}],"supportedstandards":[],"trusts":[],"extra":null} \ No newline at end of file diff --git a/internal/contracts/oracle_contract/oracle.nef b/internal/contracts/oracle_contract/oracle.nef index c7f91865fa387344ca8f5d2617b228ec9fc8a553..c088eb37555262b83817a87cb79cbc51ccfe020f 100644 GIT binary patch delta 251 zcmaFKc$s-ZH}hXd#)&v-7GGV?NvGvu>F9FuCJgIpbM_6KhU0A|2pg8%>k delta 16 Ycmcc2{E~4(H)G_)zDSlSwL6Xh06MJ)K>z>% diff --git a/internal/contracts/oracle_contract/oracle.yml b/internal/contracts/oracle_contract/oracle.yml index 1266786f9..e2d84080a 100644 --- a/internal/contracts/oracle_contract/oracle.yml +++ b/internal/contracts/oracle_contract/oracle.yml @@ -2,5 +2,6 @@ name: "Oracle test" sourceurl: https://github.com/nspcc-dev/neo-go/ supportedstandards: [] events: + - name: Invocation permissions: - - methods: ["request"] + - methods: ["request", "finish"] diff --git a/pkg/core/native/native_test/oracle_test.go b/pkg/core/native/native_test/oracle_test.go index bc51d6de9..bc32eadde 100644 --- a/pkg/core/native/native_test/oracle_test.go +++ b/pkg/core/native/native_test/oracle_test.go @@ -15,6 +15,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/neotest" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/stretchr/testify/require" ) @@ -151,6 +152,15 @@ func TestOracle_Request(t *testing.T) { require.Error(t, err) require.True(t, strings.Contains(err.Error(), "oracle tx points to invalid request")) }) + t.Run("Reentrant", func(t *testing.T) { + putOracleRequest(t, helperValidatorInvoker, "url", nil, "handleRecursive", []byte{}, gasForResponse) + tx := prepareResponseTx(t, 2) + e.AddNewBlock(t, tx) + // e.CheckFault(t, tx.Hash(), "") + aer, err := e.Chain.GetAppExecResults(tx.Hash(), trigger.Application) + require.NoError(t, err) + require.Equal(t, 2, len(aer[0].Events)) // OracleResponse + Invocation + }) t.Run("BadRequest", func(t *testing.T) { t.Run("non-UTF8 url", func(t *testing.T) { putOracleRequest(t, helperValidatorInvoker, "\xff", nil, "", []byte{1, 2}, gasForResponse, "invalid value: not UTF-8") From bc3bffea53a8302abf28301f333e9eaf7abf26da Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 22 Jul 2022 12:35:03 +0300 Subject: [PATCH 3/3] native: fix oracle.finish reentrancy bug See neo-project/neo#2795. --- pkg/core/native/native_test/oracle_test.go | 2 +- pkg/core/native/oracle.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/core/native/native_test/oracle_test.go b/pkg/core/native/native_test/oracle_test.go index bc32eadde..dec7ab41d 100644 --- a/pkg/core/native/native_test/oracle_test.go +++ b/pkg/core/native/native_test/oracle_test.go @@ -156,7 +156,7 @@ func TestOracle_Request(t *testing.T) { putOracleRequest(t, helperValidatorInvoker, "url", nil, "handleRecursive", []byte{}, gasForResponse) tx := prepareResponseTx(t, 2) e.AddNewBlock(t, tx) - // e.CheckFault(t, tx.Hash(), "") + e.CheckFault(t, tx.Hash(), "Oracle.finish called from non-entry script") aer, err := e.Chain.GetAppExecResults(tx.Hash(), trigger.Application) require.NoError(t, err) require.Equal(t, 2, len(aer[0].Events)) // OracleResponse + Invocation diff --git a/pkg/core/native/oracle.go b/pkg/core/native/oracle.go index 62ac80b00..330e640a7 100644 --- a/pkg/core/native/oracle.go +++ b/pkg/core/native/oracle.go @@ -276,6 +276,12 @@ func (o *Oracle) finish(ic *interop.Context, _ []stackitem.Item) stackitem.Item // FinishInternal processes an oracle response. func (o *Oracle) FinishInternal(ic *interop.Context) error { + if ic.VM.Istack().Len() != 2 { + return errors.New("Oracle.finish called from non-entry script") + } + if ic.Invocations[o.Hash] != 1 { + return errors.New("Oracle.finish called multiple times") + } resp := getResponse(ic.Tx) if resp == nil { return ErrResponseNotFound