From 966111f4a8312a0620097c4a00e3df20a86c2664 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 20 Jul 2023 10:55:11 +0300 Subject: [PATCH] network: forbid Notary contract to be a sender of main transaction This prevents the possible attack on notary request sender when malicious partie is allowed to send notary request with main transaction being someone else's fallback. Signed-off-by: Anna Shaleva --- pkg/network/server.go | 3 +++ pkg/network/server_test.go | 12 +++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/network/server.go b/pkg/network/server.go index 5a8f7027d..8f97665cd 100644 --- a/pkg/network/server.go +++ b/pkg/network/server.go @@ -1201,6 +1201,9 @@ func (s *Server) verifyNotaryRequest(_ *transaction.Transaction, data any) error if r.FallbackTransaction.Sender() != notaryHash { return fmt.Errorf("P2PNotary contract should be a sender of the fallback transaction, got %s", address.Uint160ToString(r.FallbackTransaction.Sender())) } + if r.MainTransaction.Sender() == notaryHash { + return errors.New("P2PNotary contract is not allowed to be the sender of the main transaction") + } depositExpiration := s.chain.GetNotaryDepositExpiration(payer) if r.FallbackTransaction.ValidUntilBlock >= depositExpiration { return fmt.Errorf("fallback transaction is valid after deposit is unlocked: ValidUntilBlock is %d, deposit lock for %s expires at %d", r.FallbackTransaction.ValidUntilBlock, address.Uint160ToString(payer), depositExpiration) diff --git a/pkg/network/server_test.go b/pkg/network/server_test.go index 71849a82f..1f862d105 100644 --- a/pkg/network/server_test.go +++ b/pkg/network/server_test.go @@ -1036,7 +1036,10 @@ func TestVerifyNotaryRequest(t *testing.T) { require.NoError(t, err) newNotaryRequest := func() *payload.P2PNotaryRequest { return &payload.P2PNotaryRequest{ - MainTransaction: &transaction.Transaction{Script: []byte{0, 1, 2}}, + MainTransaction: &transaction.Transaction{ + Script: []byte{0, 1, 2}, + Signers: []transaction.Signer{{Account: random.Uint160()}}, + }, FallbackTransaction: &transaction.Transaction{ ValidUntilBlock: 321, Signers: []transaction.Signer{{Account: bc.NotaryContractScriptHash}, {Account: random.Uint160()}}, @@ -1057,6 +1060,13 @@ func TestVerifyNotaryRequest(t *testing.T) { require.Error(t, s.verifyNotaryRequest(nil, r)) }) + t.Run("bad main sender", func(t *testing.T) { + bc.VerifyWitnessF = func() (int64, error) { return 0, nil } + r := newNotaryRequest() + r.MainTransaction.Signers[0] = transaction.Signer{Account: bc.NotaryContractScriptHash} + require.Error(t, s.verifyNotaryRequest(nil, r)) + }) + t.Run("expired deposit", func(t *testing.T) { r := newNotaryRequest() bc.NotaryDepositExpiration = r.FallbackTransaction.ValidUntilBlock