From 560e470484ab7711369818b94100324ff3f5bebd Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 2 Oct 2020 18:15:16 +0300 Subject: [PATCH] core: add validation to native Policy methods close #1442 --- pkg/core/native/policy.go | 29 +++++++++++++++++++---------- pkg/core/native_policy_test.go | 32 +++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 5a172fbbb..80fa90c3e 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -8,6 +8,7 @@ import ( "sort" "sync" + "github.com/nspcc-dev/neo-go/pkg/core/block" "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" @@ -30,6 +31,8 @@ const ( defaultMaxBlockSystemFee = 9000 * GASFactor // minBlockSystemFee is the minimum allowed system fee per block. minBlockSystemFee = 4007600 + // maxFeePerByte is the maximum allowed fee per byte value. + maxFeePerByte = 100_000_000 ) var ( @@ -311,6 +314,10 @@ func (p *Policy) GetBlockedAccountsInternal(dao dao.DAO) (BlockedAccounts, error // setMaxTransactionsPerBlock is Policy contract method and sets the upper limit // of transactions per block. func (p *Policy) setMaxTransactionsPerBlock(ic *interop.Context, args []stackitem.Item) stackitem.Item { + value := uint32(toBigInt(args[0]).Int64()) + if value > block.MaxTransactionsPerBlock { + panic(fmt.Errorf("MaxTransactionsPerBlock cannot exceed the maximum allowed transactions per block = %d", block.MaxTransactionsPerBlock)) + } ok, err := p.checkValidators(ic) if err != nil { panic(err) @@ -318,7 +325,6 @@ func (p *Policy) setMaxTransactionsPerBlock(ic *interop.Context, args []stackite if !ok { return stackitem.NewBool(false) } - value := uint32(toBigInt(args[0]).Int64()) p.lock.Lock() defer p.lock.Unlock() err = p.setUint32WithKey(ic.DAO, maxTransactionsPerBlockKey, value) @@ -331,6 +337,10 @@ func (p *Policy) setMaxTransactionsPerBlock(ic *interop.Context, args []stackite // setMaxBlockSize is Policy contract method and sets maximum block size. func (p *Policy) setMaxBlockSize(ic *interop.Context, args []stackitem.Item) stackitem.Item { + value := uint32(toBigInt(args[0]).Int64()) + if value > payload.MaxSize { + panic(fmt.Errorf("MaxBlockSize cannot be more than the maximum payload size = %d", payload.MaxSize)) + } ok, err := p.checkValidators(ic) if err != nil { panic(err) @@ -338,10 +348,6 @@ func (p *Policy) setMaxBlockSize(ic *interop.Context, args []stackitem.Item) sta if !ok { return stackitem.NewBool(false) } - value := uint32(toBigInt(args[0]).Int64()) - if payload.MaxSize <= value { - return stackitem.NewBool(false) - } p.lock.Lock() defer p.lock.Unlock() err = p.setUint32WithKey(ic.DAO, maxBlockSizeKey, value) @@ -354,6 +360,10 @@ func (p *Policy) setMaxBlockSize(ic *interop.Context, args []stackitem.Item) sta // setFeePerByte is Policy contract method and sets transaction's fee per byte. func (p *Policy) setFeePerByte(ic *interop.Context, args []stackitem.Item) stackitem.Item { + value := toBigInt(args[0]).Int64() + if value < 0 || value > maxFeePerByte { + panic(fmt.Errorf("FeePerByte shouldn't be negative or greater than %d", maxFeePerByte)) + } ok, err := p.checkValidators(ic) if err != nil { panic(err) @@ -361,7 +371,6 @@ func (p *Policy) setFeePerByte(ic *interop.Context, args []stackitem.Item) stack if !ok { return stackitem.NewBool(false) } - value := toBigInt(args[0]).Int64() p.lock.Lock() defer p.lock.Unlock() err = p.setInt64WithKey(ic.DAO, feePerByteKey, value) @@ -374,6 +383,10 @@ func (p *Policy) setFeePerByte(ic *interop.Context, args []stackitem.Item) stack // setMaxBlockSystemFee is Policy contract method and sets the maximum system fee per block. func (p *Policy) setMaxBlockSystemFee(ic *interop.Context, args []stackitem.Item) stackitem.Item { + value := toBigInt(args[0]).Int64() + if value <= minBlockSystemFee { + panic(fmt.Errorf("MaxBlockSystemFee cannot be less then %d", minBlockSystemFee)) + } ok, err := p.checkValidators(ic) if err != nil { panic(err) @@ -381,10 +394,6 @@ func (p *Policy) setMaxBlockSystemFee(ic *interop.Context, args []stackitem.Item if !ok { return stackitem.NewBool(false) } - value := toBigInt(args[0]).Int64() - if value <= minBlockSystemFee { - return stackitem.NewBool(false) - } p.lock.Lock() defer p.lock.Unlock() err = p.setInt64WithKey(ic.DAO, maxBlockSystemFeeKey, value) diff --git a/pkg/core/native_policy_test.go b/pkg/core/native_policy_test.go index 2fb56c8c9..9a5c9f777 100644 --- a/pkg/core/native_policy_test.go +++ b/pkg/core/native_policy_test.go @@ -5,11 +5,13 @@ import ( "sort" "testing" + "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/core/native" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/encoding/bigint" "github.com/nspcc-dev/neo-go/pkg/io" + "github.com/nspcc-dev/neo-go/pkg/network/payload" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" @@ -41,6 +43,12 @@ func TestMaxTransactionsPerBlock(t *testing.T) { n := chain.contracts.Policy.GetMaxTransactionsPerBlockInternal(chain.dao) require.Equal(t, 1024, int(n)) }) + + t.Run("set, too big value", func(t *testing.T) { + res, err := invokeNativePolicyMethod(chain, "setMaxTransactionsPerBlock", bigint.ToBytes(big.NewInt(block.MaxContentsPerBlock))) + require.NoError(t, err) + checkFAULTState(t, res) + }) } func TestMaxBlockSize(t *testing.T) { @@ -69,6 +77,12 @@ func TestMaxBlockSize(t *testing.T) { checkResult(t, res, stackitem.NewBigInteger(big.NewInt(102400))) require.NoError(t, chain.persist()) }) + + t.Run("set, too big value", func(t *testing.T) { + res, err := invokeNativePolicyMethod(chain, "setMaxBlockSize", bigint.ToBytes(big.NewInt(payload.MaxSize+1))) + require.NoError(t, err) + checkFAULTState(t, res) + }) } func TestFeePerByte(t *testing.T) { @@ -95,6 +109,18 @@ func TestFeePerByte(t *testing.T) { n := chain.contracts.Policy.GetFeePerByteInternal(chain.dao) require.Equal(t, 1024, int(n)) }) + + t.Run("set, negative value", func(t *testing.T) { + res, err := invokeNativePolicyMethod(chain, "setFeePerByte", bigint.ToBytes(big.NewInt(-1))) + require.NoError(t, err) + checkFAULTState(t, res) + }) + + t.Run("set, too big value", func(t *testing.T) { + res, err := invokeNativePolicyMethod(chain, "setFeePerByte", bigint.ToBytes(big.NewInt(100_000_000+1))) + require.NoError(t, err) + checkFAULTState(t, res) + }) } func TestBlockSystemFee(t *testing.T) { @@ -116,7 +142,7 @@ func TestBlockSystemFee(t *testing.T) { t.Run("set, too low fee", func(t *testing.T) { res, err := invokeNativePolicyMethod(chain, "setMaxBlockSystemFee", bigint.ToBytes(big.NewInt(4007600))) require.NoError(t, err) - checkResult(t, res, stackitem.NewBool(false)) + checkFAULTState(t, res) }) t.Run("set, success", func(t *testing.T) { @@ -251,3 +277,7 @@ func checkResult(t *testing.T, result *state.AppExecResult, expected stackitem.I require.Equal(t, 1, len(result.Stack)) require.Equal(t, expected, result.Stack[0]) } + +func checkFAULTState(t *testing.T, result *state.AppExecResult) { + require.Equal(t, vm.FaultState, result.VMState) +}