From b38126ba3ea752a1020cc487a4c61da3a468a13e Mon Sep 17 00:00:00 2001 From: Pavel Karpy Date: Tue, 25 Jul 2023 20:38:43 +0300 Subject: [PATCH] preparator: Skip unexpected TXs Notary requests can be sent by anybody who has GAS and knows how an Alphabet node parses them. Add predefined allowed events on notary request parsing level. Signed-off-by: Pavel Karpy --- pkg/morph/event/listener.go | 2 ++ pkg/morph/event/notary_preparator.go | 33 +++++++++++++++++++- pkg/morph/event/notary_preparator_test.go | 38 ++++++++++++++++++++--- pkg/morph/event/utils.go | 4 +++ 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/pkg/morph/event/listener.go b/pkg/morph/event/listener.go index d077e756193..497b5928c02 100644 --- a/pkg/morph/event/listener.go +++ b/pkg/morph/event/listener.go @@ -516,6 +516,8 @@ func (l *listener) SetNotaryParser(pi NotaryParserInfo) { l.notaryParsers[pi.notaryRequestTypes] = pi.parser() } + l.notaryEventsPreparator.allowNotaryEvent(pi.notaryScriptWithHash) + log.Info("registered new event parser") } diff --git a/pkg/morph/event/notary_preparator.go b/pkg/morph/event/notary_preparator.go index cb739201e62..3b17c34318e 100644 --- a/pkg/morph/event/notary_preparator.go +++ b/pkg/morph/event/notary_preparator.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "errors" "fmt" + "sync" lru "github.com/hashicorp/golang-lru/v2" "github.com/nspcc-dev/neo-go/pkg/core/interop/interopnames" @@ -43,6 +44,9 @@ var ( // ErrMainTXExpired is returned if received fallback TX is already valid. ErrMainTXExpired = errors.New("received main tx has expired") + + // ErrUnknownEvent is returned if main transaction is not expected. + ErrUnknownEvent = errors.New("received notary request is not allowed") ) // BlockCounter must return block count of the network @@ -76,6 +80,9 @@ type preparator struct { // just an optimization, already handled TX are not gonna // be signed once more anyway alreadyHandledTXs *lru.Cache[util.Uint256, struct{}] + + m *sync.RWMutex + allowedEvents map[notaryScriptWithHash]struct{} } // notaryPreparator inits and returns preparator. @@ -104,6 +111,8 @@ func notaryPreparator(prm PreparatorPrm) preparator { alphaKeys: prm.AlphaKeys, blockCounter: prm.BlockCounter, alreadyHandledTXs: cache, + m: &sync.RWMutex{}, + allowedEvents: make(map[notaryScriptWithHash]struct{}), } } @@ -114,6 +123,9 @@ func notaryPreparator(prm PreparatorPrm) preparator { // transaction is expected to be received one more time // from the Notary service but already signed. This happens // since every notary call is a new notary request in fact. +// +// Returns ErrUnknownEvent if main transactions is not an +// expected contract call. func (p preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) { if _, ok := p.alreadyHandledTXs.Get(nr.MainTransaction.Hash()); ok { // received already signed and sent TX @@ -208,6 +220,18 @@ func (p preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) { // retrieve contract's method contractMethod := string(ops[opsLen-3].param) + eventType := NotaryTypeFromString(contractMethod) + + p.m.RLock() + _, allowed := p.allowedEvents[notaryScriptWithHash{ + scriptHashValue: scriptHashValue{contractHash}, + notaryRequestType: notaryRequestType{eventType}, + }] + p.m.RUnlock() + + if !allowed { + return nil, ErrUnknownEvent + } // check if there is a call flag(must be in range [0:15)) callFlag := callflag.CallFlag(ops[opsLen-4].code - opcode.PUSH0) @@ -231,12 +255,19 @@ func (p preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) { return parsedNotaryEvent{ hash: contractHash, - notaryType: NotaryTypeFromString(contractMethod), + notaryType: eventType, params: args, raw: nr, }, nil } +func (p preparator) allowNotaryEvent(filter notaryScriptWithHash) { + p.m.Lock() + defer p.m.Unlock() + + p.allowedEvents[filter] = struct{}{} +} + func (p preparator) validateParameterOpcodes(ops []Op) error { l := len(ops) diff --git a/pkg/morph/event/notary_preparator_test.go b/pkg/morph/event/notary_preparator_test.go index 64245dcdc81..735784e9055 100644 --- a/pkg/morph/event/notary_preparator_test.go +++ b/pkg/morph/event/notary_preparator_test.go @@ -2,6 +2,7 @@ package event import ( "fmt" + "strconv" "testing" "github.com/nspcc-dev/neo-go/pkg/vm" @@ -21,6 +22,8 @@ import ( "github.com/stretchr/testify/require" ) +const contractMethod = "test" + var ( alphaKeys keys.PublicKeys wrongAlphaKeys keys.PublicKeys @@ -63,12 +66,17 @@ func TestPrepare_IncorrectScript(t *testing.T) { }, ) + preparator.allowNotaryEvent(notaryScriptWithHash{ + notaryRequestType: notaryRequestType{contractMethod}, + scriptHashValue: scriptHashValue{scriptHash}, + }) + for _, dummyMultisig := range []bool{true, false} { // try both empty and dummy multisig/Notary invocation witness script t.Run(fmt.Sprintf("not contract call, compat: %t", dummyMultisig), func(t *testing.T) { bw := io.NewBufBinWriter() emit.Int(bw.BinWriter, 4) - emit.String(bw.BinWriter, "test") + emit.String(bw.BinWriter, contractMethod) emit.Bytes(bw.BinWriter, scriptHash.BytesBE()) emit.Syscall(bw.BinWriter, interopnames.SystemContractCallNative) // any != interopnames.SystemContractCall @@ -83,7 +91,7 @@ func TestPrepare_IncorrectScript(t *testing.T) { bw := io.NewBufBinWriter() emit.Int(bw.BinWriter, -1) - emit.String(bw.BinWriter, "test") + emit.String(bw.BinWriter, contractMethod) emit.Bytes(bw.BinWriter, scriptHash.BytesBE()) emit.Syscall(bw.BinWriter, interopnames.SystemContractCall) @@ -448,14 +456,20 @@ func TestPrepare_CorrectNR(t *testing.T) { for _, test := range tests { for i := 0; i < 1; i++ { // run tests against 3 and 4 witness NR - for _, dummyMultisig := range []bool{true, false} { // run tests against empty and dummy multisig/Notary witness + for j, dummyMultisig := range []bool{true, false} { // run tests against empty and dummy multisig/Notary witness + method := test.method + strconv.FormatInt(int64(j), 10) + preparator.allowNotaryEvent(notaryScriptWithHash{ + notaryRequestType: notaryRequestType{NotaryTypeFromString(method)}, + scriptHashValue: scriptHashValue{scriptHash}, + }) + additionalWitness := i == 0 - nr := correctNR(script(test.hash, test.method, test.args...), dummyMultisig, additionalWitness) + nr := correctNR(script(test.hash, method, test.args...), dummyMultisig, additionalWitness) event, err := preparator.Prepare(nr) require.NoError(t, err) - require.Equal(t, test.method, event.Type().String()) + require.Equal(t, method, event.Type().String()) require.Equal(t, test.hash.StringLE(), event.ScriptHash().StringLE()) // check args parsing @@ -488,6 +502,20 @@ func TestPrepare_CorrectNR(t *testing.T) { } } +func TestNotAllowedEvents(t *testing.T) { + preparator := notaryPreparator( + PreparatorPrm{ + alphaKeysSource(), + blockCounter{100, nil}, + }, + ) + + nr := correctNR(script(scriptHash, "test1", nil), false, false) + + _, err := preparator.Prepare(nr) + require.ErrorIs(t, err, ErrUnknownEvent) +} + func alphaKeysSource() client.AlphabetKeys { return func() (keys.PublicKeys, error) { return alphaKeys, nil diff --git a/pkg/morph/event/utils.go b/pkg/morph/event/utils.go index 5e6a3370162..3777cdbade6 100644 --- a/pkg/morph/event/utils.go +++ b/pkg/morph/event/utils.go @@ -29,6 +29,10 @@ type scriptHashWithType struct { type notaryRequestTypes struct { notaryRequestMempoolType + notaryScriptWithHash +} + +type notaryScriptWithHash struct { notaryRequestType scriptHashValue }