From c63289a564c88965c3c7806782ff40416e43a8e3 Mon Sep 17 00:00:00 2001
From: Anna Shaleva <shaleva.ann@nspcc.ru>
Date: Tue, 29 Aug 2023 14:40:51 +0300
Subject: [PATCH] notary: avoid changing pooled fallback transaction witnesses

Forbid Notary service to change the fallback's witnesses in any way.
Fix the problem described in review comment:
https://github.com/nspcc-dev/neo-go/pull/3098#discussion_r1308336339.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
---
 pkg/services/notary/notary.go | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/pkg/services/notary/notary.go b/pkg/services/notary/notary.go
index d2a47e2ae..995cd80e9 100644
--- a/pkg/services/notary/notary.go
+++ b/pkg/services/notary/notary.go
@@ -19,6 +19,7 @@ import (
 	"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/util/slice"
 	"github.com/nspcc-dev/neo-go/pkg/vm"
 	"github.com/nspcc-dev/neo-go/pkg/vm/opcode"
 	"github.com/nspcc-dev/neo-go/pkg/wallet"
@@ -264,11 +265,8 @@ func (n *Notary) OnNewRequest(payload *payload.P2PNotaryRequest) {
 	} else {
 		// Avoid changes in the main transaction witnesses got from the notary request pool to
 		// keep the pooled tx valid. We will update its copy => the copy's size will be changed.
-		cp := *payload.MainTransaction
-		cp.Scripts = make([]transaction.Witness, len(payload.MainTransaction.Scripts))
-		copy(cp.Scripts, payload.MainTransaction.Scripts)
 		r = &request{
-			main:              &cp,
+			main:              safeCopy(payload.MainTransaction),
 			minNotValidBefore: nvbFallback,
 		}
 		n.requests[payload.MainTransaction.Hash()] = r
@@ -276,9 +274,12 @@ func (n *Notary) OnNewRequest(payload *payload.P2PNotaryRequest) {
 	if r.witnessInfo == nil && validationErr == nil {
 		r.witnessInfo = newInfo
 	}
-	// Allow modification of a fallback transaction got from the notary request pool.
-	// It has dummy Notary witness attached => its size won't be changed.
-	r.fallbacks = append(r.fallbacks, payload.FallbackTransaction)
+	// Disallow modification of a fallback transaction got from the notary
+	// request pool. Even though it has dummy Notary witness attached and its
+	// size won't be changed after finalisation, the witness bytes changes may
+	// affect the other users of notary pool and cause race. Avoid this by making
+	// the copy.
+	r.fallbacks = append(r.fallbacks, safeCopy(payload.FallbackTransaction))
 	if exists && r.isMainCompleted() || validationErr != nil {
 		return
 	}
@@ -338,6 +339,21 @@ func (n *Notary) OnNewRequest(payload *payload.P2PNotaryRequest) {
 	}
 }
 
+// safeCopy creates a copy of provided transaction by dereferencing it and creating
+// fresh witnesses so that the tx's witnesses may be modified without affecting the
+// copy's ones.
+func safeCopy(tx *transaction.Transaction) *transaction.Transaction {
+	cp := *tx
+	cp.Scripts = make([]transaction.Witness, len(tx.Scripts))
+	for i := range cp.Scripts {
+		cp.Scripts[i] = transaction.Witness{
+			InvocationScript:   slice.Copy(tx.Scripts[i].InvocationScript),
+			VerificationScript: slice.Copy(tx.Scripts[i].VerificationScript),
+		}
+	}
+	return &cp
+}
+
 // OnRequestRemoval is a callback which is called after fallback transaction is removed
 // from the notary payload pool due to expiration, main tx appliance or any other reason.
 func (n *Notary) OnRequestRemoval(pld *payload.P2PNotaryRequest) {