Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce stackitem serialization limits #3218

Merged
merged 4 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/smartcontract/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func readManifest(filename string, hash util.Uint160) (*manifest.Manifest, []byt
if err != nil {
return nil, nil, err
}
if err := m.IsValid(hash); err != nil {
if err := m.IsValid(hash, true); err != nil {
return nil, nil, fmt.Errorf("manifest is invalid: %w", err)
}
return m, manifestBytes, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ func CreateManifest(di *DebugInfo, o *Options) (*manifest.Manifest, error) {
return m, fmt.Errorf("method %s is marked as safe but missing from manifest", name)
}
}
err = m.IsValid(util.Uint160{}) // Check as much as possible without hash.
err = m.IsValid(util.Uint160{}, true) // Check as much as possible without hash.
if err != nil {
return m, fmt.Errorf("manifest is invalid: %w", err)
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/core/blockchain_neotest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ func TestBlockchain_StartFromExistingDB(t *testing.T) {
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), "can't init MPT at height"), err)
})
/* See #2801
t.Run("failed native Management initialisation", func(t *testing.T) {
ps = newPS(t)

Expand All @@ -234,9 +233,8 @@ func TestBlockchain_StartFromExistingDB(t *testing.T) {

_, _, _, err := chain.NewMultiWithCustomConfigAndStoreNoCheck(t, customConfig, cache)
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), "can't init cache for Management native contract"), err)
require.True(t, strings.Contains(err.Error(), "can't init natives cache: failed to initialize cache for ContractManagement"), err)
})
*/
t.Run("invalid native contract activation", func(t *testing.T) {
ps = newPS(t)

Expand Down
16 changes: 11 additions & 5 deletions pkg/core/native/management.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ func (m *Management) Deploy(ic *interop.Context, sender util.Uint160, neff *nef.
if err != nil {
return nil, err
}
err = manif.IsValid(h)
err = manif.IsValid(h, false) // do not check manifest size, the whole state.Contract will be checked later.
if err != nil {
return nil, fmt.Errorf("invalid manifest: %w", err)
}
Expand Down Expand Up @@ -458,7 +458,7 @@ func (m *Management) Update(ic *interop.Context, hash util.Uint160, neff *nef.Fi
if manif.Name != contract.Manifest.Name {
return nil, errors.New("contract name can't be changed")
}
err = manif.IsValid(contract.Hash)
err = manif.IsValid(contract.Hash, false) // do not check manifest size, the whole state.Contract will be checked later.
if err != nil {
return nil, fmt.Errorf("invalid manifest: %w", err)
}
Expand Down Expand Up @@ -619,13 +619,19 @@ func (m *Management) InitializeCache(blockHeight uint32, d *dao.Simple) error {
nep17: make(map[util.Uint160]struct{}),
}

d.Seek(m.ID, storage.SeekRange{Prefix: []byte{PrefixContract}}, func(k, v []byte) bool {
var initErr error
d.Seek(m.ID, storage.SeekRange{Prefix: []byte{PrefixContract}}, func(_, v []byte) bool {
var cs = new(state.Contract)
if stackitem.DeserializeConvertible(v, cs) == nil {
updateContractCache(cache, cs)
initErr = stackitem.DeserializeConvertible(v, cs)
if initErr != nil {
return false
}
updateContractCache(cache, cs)
return true
})
if initErr != nil {
return initErr
}
d.SetCache(m.ID, cache)
return nil
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/core/native/management_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,12 @@ func TestManagement_Initialize(t *testing.T) {
mgmt := newManagement()
require.NoError(t, mgmt.InitializeCache(0, d))
})
/* See #2801
t.Run("invalid contract state", func(t *testing.T) {
d := dao.NewSimple(storage.NewMemoryStore(), false)
mgmt := newManagement()
d.PutStorageItem(mgmt.ID, []byte{PrefixContract}, state.StorageItem{0xFF})
require.Error(t, mgmt.InitializeCache(d))
require.Error(t, mgmt.InitializeCache(0, d))
})
*/
}

func TestManagement_GetNEP17Contracts(t *testing.T) {
Expand Down
19 changes: 17 additions & 2 deletions pkg/smartcontract/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (m *Manifest) CanCall(hash util.Uint160, toCall *Manifest, method string) b
// IsValid checks manifest internal consistency and correctness, one of the
// checks is for group signature correctness, contract hash is passed for it.
// If hash is empty, then hash-related checks are omitted.
func (m *Manifest) IsValid(hash util.Uint160) error {
func (m *Manifest) IsValid(hash util.Uint160, checkSize bool) error {
var err error

if m.Name == "" {
Expand Down Expand Up @@ -118,7 +118,22 @@ func (m *Manifest) IsValid(hash util.Uint160) error {
return errors.New("duplicate trusted contracts")
}
}
return Permissions(m.Permissions).AreValid()
err = Permissions(m.Permissions).AreValid()
if err != nil {
return err
}
if !checkSize {
return nil
}
si, err := m.ToStackItem()
if err != nil {
return fmt.Errorf("failed to check manifest serialisation: %w", err)
}
_, err = stackitem.Serialize(si)
roman-khimov marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return fmt.Errorf("manifest is not serializable: %w", err)
}
return nil
}

// IsStandardSupported denotes whether the specified standard is supported by the contract.
Expand Down
45 changes: 29 additions & 16 deletions pkg/smartcontract/manifest/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package manifest

import (
"encoding/json"
"fmt"
"math/big"
"testing"

Expand Down Expand Up @@ -124,13 +125,13 @@ func TestIsValid(t *testing.T) {
m := &Manifest{}

t.Run("invalid, no name", func(t *testing.T) {
require.Error(t, m.IsValid(contractHash))
require.Error(t, m.IsValid(contractHash, true))
})

m = NewManifest("Test")

t.Run("invalid, no ABI methods", func(t *testing.T) {
require.Error(t, m.IsValid(contractHash))
require.Error(t, m.IsValid(contractHash, true))
})

m.ABI.Methods = append(m.ABI.Methods, Method{
Expand All @@ -140,7 +141,7 @@ func TestIsValid(t *testing.T) {
})

t.Run("valid, no groups/events", func(t *testing.T) {
require.NoError(t, m.IsValid(contractHash))
require.NoError(t, m.IsValid(contractHash, true))
})

m.ABI.Events = append(m.ABI.Events, Event{
Expand All @@ -149,7 +150,7 @@ func TestIsValid(t *testing.T) {
})

t.Run("valid, with events", func(t *testing.T) {
require.NoError(t, m.IsValid(contractHash))
require.NoError(t, m.IsValid(contractHash, true))
})

m.ABI.Events = append(m.ABI.Events, Event{
Expand All @@ -161,52 +162,52 @@ func TestIsValid(t *testing.T) {
})

t.Run("invalid, bad event", func(t *testing.T) {
require.Error(t, m.IsValid(contractHash))
require.Error(t, m.IsValid(contractHash, true))
})
m.ABI.Events = m.ABI.Events[:1]

m.Permissions = append(m.Permissions, *NewPermission(PermissionHash, util.Uint160{1, 2, 3}))
t.Run("valid, with permissions", func(t *testing.T) {
require.NoError(t, m.IsValid(contractHash))
require.NoError(t, m.IsValid(contractHash, true))
})

m.Permissions = append(m.Permissions, *NewPermission(PermissionHash, util.Uint160{1, 2, 3}))
t.Run("invalid, with permissions", func(t *testing.T) {
require.Error(t, m.IsValid(contractHash))
require.Error(t, m.IsValid(contractHash, true))
})
m.Permissions = m.Permissions[:1]

m.SupportedStandards = append(m.SupportedStandards, "NEP-17")
t.Run("valid, with standards", func(t *testing.T) {
require.NoError(t, m.IsValid(contractHash))
require.NoError(t, m.IsValid(contractHash, true))
})

m.SupportedStandards = append(m.SupportedStandards, "")
t.Run("invalid, with nameless standard", func(t *testing.T) {
require.Error(t, m.IsValid(contractHash))
require.Error(t, m.IsValid(contractHash, true))
})
m.SupportedStandards = m.SupportedStandards[:1]

m.SupportedStandards = append(m.SupportedStandards, "NEP-17")
t.Run("invalid, with duplicate standards", func(t *testing.T) {
require.Error(t, m.IsValid(contractHash))
require.Error(t, m.IsValid(contractHash, true))
})
m.SupportedStandards = m.SupportedStandards[:1]

d := PermissionDesc{Type: PermissionHash, Value: util.Uint160{1, 2, 3}}
m.Trusts.Add(d)
t.Run("valid, with trust", func(t *testing.T) {
require.NoError(t, m.IsValid(contractHash))
require.NoError(t, m.IsValid(contractHash, true))
})

m.Trusts.Add(PermissionDesc{Type: PermissionHash, Value: util.Uint160{3, 2, 1}})
t.Run("valid, with trusts", func(t *testing.T) {
require.NoError(t, m.IsValid(contractHash))
require.NoError(t, m.IsValid(contractHash, true))
})

m.Trusts.Add(d)
t.Run("invalid, with trusts", func(t *testing.T) {
require.Error(t, m.IsValid(contractHash))
require.Error(t, m.IsValid(contractHash, true))
})
m.Trusts.Restrict()

Expand All @@ -224,11 +225,11 @@ func TestIsValid(t *testing.T) {
}

t.Run("valid", func(t *testing.T) {
require.NoError(t, m.IsValid(contractHash))
require.NoError(t, m.IsValid(contractHash, true))
})

t.Run("invalid, wrong contract hash", func(t *testing.T) {
require.Error(t, m.IsValid(util.Uint160{4, 5, 6}))
require.Error(t, m.IsValid(util.Uint160{4, 5, 6}, true))
})

t.Run("invalid, wrong group signature", func(t *testing.T) {
Expand All @@ -240,9 +241,21 @@ func TestIsValid(t *testing.T) {
// of the contract hash.
Signature: pk.Sign([]byte{1, 2, 3}),
})
require.Error(t, m.IsValid(contractHash))
require.Error(t, m.IsValid(contractHash, true))
})
})
m.Groups = m.Groups[:0]

t.Run("invalid, unserializable", func(t *testing.T) {
for i := 0; i < stackitem.MaxSerialized; i++ {
m.ABI.Events = append(m.ABI.Events, Event{
Name: fmt.Sprintf("Event%d", i),
Parameters: []Parameter{},
})
}
err := m.IsValid(contractHash, true)
require.ErrorIs(t, err, stackitem.ErrTooBig)
})
}

func TestManifestToStackItem(t *testing.T) {
Expand Down
13 changes: 8 additions & 5 deletions pkg/vm/stackitem/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,12 @@ func ToJSON(item Item) ([]byte, error) {
}

// sliceNoPointer represents a sub-slice of a known slice.
// It doesn't contain any pointer and uses less memory than `[]byte`.
// It doesn't contain any pointer and uses the same amount of memory as `[]byte`,
// but at the same type has additional information about the number of items in
// the stackitem (including the stackitem itself).
type sliceNoPointer struct {
start, end int
itemsCount int
roman-khimov marked this conversation as resolved.
Show resolved Hide resolved
}

func toJSON(data []byte, seen map[Item]sliceNoPointer, item Item) ([]byte, error) {
Expand Down Expand Up @@ -105,7 +108,7 @@ func toJSON(data []byte, seen map[Item]sliceNoPointer, item Item) ([]byte, error
}
}
data = append(data, ']')
seen[item] = sliceNoPointer{start, len(data)}
seen[item] = sliceNoPointer{start: start, end: len(data)}
case *Map:
data = append(data, '{')
for i := range it.value {
Expand All @@ -126,7 +129,7 @@ func toJSON(data []byte, seen map[Item]sliceNoPointer, item Item) ([]byte, error
}
}
data = append(data, '}')
seen[item] = sliceNoPointer{start, len(data)}
seen[item] = sliceNoPointer{start: start, end: len(data)}
case *BigInteger:
if it.Big().CmpAbs(big.NewInt(MaxAllowedInteger)) == 1 {
return nil, fmt.Errorf("%w (MaxAllowedInteger)", ErrInvalidValue)
Expand Down Expand Up @@ -420,15 +423,15 @@ func toJSONWithTypes(data []byte, item Item, seen map[Item]sliceNoPointer) ([]by
data = append(data, '}')

if isBuffer {
seen[item] = sliceNoPointer{start, len(data)}
seen[item] = sliceNoPointer{start: start, end: len(data)}
}
} else {
if len(data)+2 > MaxSize { // also take care of '}'
return nil, errTooBigSize
}
data = append(data, ']', '}')

seen[item] = sliceNoPointer{start, len(data)}
seen[item] = sliceNoPointer{start: start, end: len(data)}
}
return data, nil
}
Expand Down
Loading
Loading