From 90705e37e9bfd8c630b02c5b366a5db68bdb14b1 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 2 Nov 2023 14:48:51 +0300 Subject: [PATCH] compiler: perform NEF size check on serialization Retun an error if the serialized NEF size exceeds stackitem.MaxSize. Signed-off-by: Anna Shaleva --- cli/smartcontract/contract_test.go | 25 +++++++++++++++++++++++++ pkg/compiler/compiler_test.go | 24 ++++++++++++++++++++++++ pkg/core/state/contract.go | 2 ++ pkg/smartcontract/nef/nef.go | 27 ++++++++++++++++++++++++--- pkg/smartcontract/nef/nef_test.go | 2 +- 5 files changed, 76 insertions(+), 4 deletions(-) diff --git a/cli/smartcontract/contract_test.go b/cli/smartcontract/contract_test.go index fca6808f3..8072d505b 100644 --- a/cli/smartcontract/contract_test.go +++ b/cli/smartcontract/contract_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/hex" "encoding/json" + "fmt" "os" "path/filepath" "strconv" @@ -1070,3 +1071,27 @@ func filterFilename(infos []os.DirEntry, ext string) string { } return "" } + +func TestContractCompile_NEFSizeCheck(t *testing.T) { + tmpDir := t.TempDir() + e := testcli.NewExecutor(t, false) + + src := `package nefconstraints + var data = "%s" + + func Main() string { + return data + }` + data := make([]byte, stackitem.MaxSize-10) + for i := range data { + data[i] = byte('a') + } + + in := filepath.Join(tmpDir, "main.go") + cfg := filepath.Join(tmpDir, "main.yml") + require.NoError(t, os.WriteFile(cfg, []byte("name: main"), os.ModePerm)) + require.NoError(t, os.WriteFile(in, []byte(fmt.Sprintf(src, data)), os.ModePerm)) + + e.RunWithError(t, "neo-go", "contract", "compile", "--in", in) + require.NoFileExists(t, filepath.Join(tmpDir, "main.nef")) +} diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go index 6cbbacc1f..9b17c6975 100644 --- a/pkg/compiler/compiler_test.go +++ b/pkg/compiler/compiler_test.go @@ -16,6 +16,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract" "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" "github.com/stretchr/testify/require" ) @@ -84,6 +85,29 @@ func TestCompiler(t *testing.T) { require.NoError(t, err) }, }, + { + name: "TestCompileAndSave_NEF_constraints", + function: func(t *testing.T) { + tmp := t.TempDir() + src := `package nefconstraints + var data = "%s" + + func Main() string { + return data + } +` + data := make([]byte, stackitem.MaxSize-10) + for i := range data { + data[i] = byte('a') + } + in := filepath.Join(tmp, "src.go") + require.NoError(t, os.WriteFile(in, []byte(fmt.Sprintf(src, data)), os.ModePerm)) + out := filepath.Join(tmp, "test.nef") + _, err := compiler.CompileAndSave(in, &compiler.Options{Outfile: out}) + require.Error(t, err) + require.Contains(t, err.Error(), "serialized NEF size exceeds VM stackitem limits") + }, + }, } for _, tcase := range testCases { diff --git a/pkg/core/state/contract.go b/pkg/core/state/contract.go index 6b5048d2c..e5db37717 100644 --- a/pkg/core/state/contract.go +++ b/pkg/core/state/contract.go @@ -37,6 +37,8 @@ type NativeContract struct { // ToStackItem converts state.Contract to stackitem.Item. func (c *Contract) ToStackItem() (stackitem.Item, error) { + // Do not skip the NEF size check, it won't affect native Management related + // states as the same checked is performed during contract deploy/update. rawNef, err := c.NEF.Bytes() if err != nil { return nil, err diff --git a/pkg/smartcontract/nef/nef.go b/pkg/smartcontract/nef/nef.go index cd7f92d13..9d8091210 100644 --- a/pkg/smartcontract/nef/nef.go +++ b/pkg/smartcontract/nef/nef.go @@ -99,8 +99,11 @@ func (h *Header) DecodeBinary(r *io.BinReader) { } // CalculateChecksum returns first 4 bytes of double-SHA256(Header) converted to uint32. +// CalculateChecksum doesn't perform the resulting serialized NEF size check, and return +// the checksum irrespectively to the size limit constraint. It's a caller's duty to check +// the resulting NEF size. func (n *File) CalculateChecksum() uint32 { - bb, err := n.Bytes() + bb, err := n.BytesLong() if err != nil { panic(err) } @@ -152,14 +155,32 @@ func (n *File) DecodeBinary(r *io.BinReader) { } } -// Bytes returns a byte array with a serialized NEF File. +// Bytes returns a byte array with a serialized NEF File. It performs the +// resulting NEF file size check and returns an error if serialized slice length +// exceeds [stackitem.MaxSize]. func (n File) Bytes() ([]byte, error) { + return n.bytes(true) +} + +// BytesLong returns a byte array with a serialized NEF File. It performs no +// resulting slice check. +func (n File) BytesLong() ([]byte, error) { + return n.bytes(false) +} + +// bytes returns the serialized NEF File representation and performs the resulting +// byte array size check if needed. +func (n File) bytes(checkSize bool) ([]byte, error) { buf := io.NewBufBinWriter() n.EncodeBinary(buf.BinWriter) if buf.Err != nil { return nil, buf.Err } - return buf.Bytes(), nil + res := buf.Bytes() + if checkSize && len(res) > stackitem.MaxSize { + return nil, fmt.Errorf("serialized NEF size exceeds VM stackitem limits: %d bytes is allowed at max, got %d", stackitem.MaxSize, len(res)) + } + return res, nil } // FileFromBytes returns a NEF File deserialized from the given bytes. diff --git a/pkg/smartcontract/nef/nef_test.go b/pkg/smartcontract/nef/nef_test.go index 9d0e70f7e..d261e7f72 100644 --- a/pkg/smartcontract/nef/nef_test.go +++ b/pkg/smartcontract/nef/nef_test.go @@ -144,7 +144,7 @@ func TestNewFileFromBytesLimits(t *testing.T) { } expected.Checksum = expected.CalculateChecksum() - bytes, err := expected.Bytes() + bytes, err := expected.BytesLong() require.NoError(t, err) _, err = FileFromBytes(bytes) require.Error(t, err)