From fc1ca4ffb94e2deb92139f9ed4d39d371c8e70a4 Mon Sep 17 00:00:00 2001
From: tbjump <unknown>
Date: Sat, 18 Feb 2023 00:19:55 +0000
Subject: [PATCH] node/accountant: defense-in-depth ensure accountant does not
 create messages outside its domain

---
 node/pkg/accountant/accountant.go | 24 ++++++++++++++++++------
 node/pkg/processor/processor.go   |  4 ++++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/node/pkg/accountant/accountant.go b/node/pkg/accountant/accountant.go
index d6d858919..cf5b01504 100644
--- a/node/pkg/accountant/accountant.go
+++ b/node/pkg/accountant/accountant.go
@@ -192,12 +192,10 @@ func (acct *Accountant) FeatureString() string {
 	return "acct:enforced"
 }
 
-// SubmitObservation will submit token bridge transfers to the accountant smart contract. This is called from the processor
-// loop when a local observation is received from a watcher. It returns true if the observation can be published immediately,
-// false if not (because it has been submitted to accountant).
-func (acct *Accountant) SubmitObservation(msg *common.MessagePublication) (bool, error) {
+// IsMessageCoveredByAccountant returns `true` if a message should be processed by the Global Accountant, `false` if not.
+func (acct *Accountant) IsMessageCoveredByAccountant(msg *common.MessagePublication) bool {
 	msgId := msg.MessageIDString()
-	acct.logger.Debug("acct: in SubmitObservation", zap.String("msgID", msgId))
+
 	// We only care about token bridges.
 	tbk := tokenBridgeKey{emitterChainId: msg.EmitterChain, emitterAddr: msg.EmitterAddress}
 	if _, exists := acct.tokenBridges[tbk]; !exists {
@@ -205,12 +203,26 @@ func (acct *Accountant) SubmitObservation(msg *common.MessagePublication) (bool,
 			acct.logger.Debug("acct: ignoring vaa because it is not a token bridge", zap.String("msgID", msgId))
 		}
 
-		return true, nil
+		return false
 	}
 
 	// We only care about transfers.
 	if !vaa.IsTransfer(msg.Payload) {
 		acct.logger.Info("acct: ignoring vaa because it is not a transfer", zap.String("msgID", msgId))
+		return false
+	}
+
+	return true
+}
+
+// SubmitObservation will submit token bridge transfers to the accountant smart contract. This is called from the processor
+// loop when a local observation is received from a watcher. It returns true if the observation can be published immediately,
+// false if not (because it has been submitted to accountant).
+func (acct *Accountant) SubmitObservation(msg *common.MessagePublication) (bool, error) {
+	msgId := msg.MessageIDString()
+	acct.logger.Debug("acct: in SubmitObservation", zap.String("msgID", msgId))
+
+	if !acct.IsMessageCoveredByAccountant(msg) {
 		return true, nil
 	}
 
diff --git a/node/pkg/processor/processor.go b/node/pkg/processor/processor.go
index 1334bbb98..710e781ce 100644
--- a/node/pkg/processor/processor.go
+++ b/node/pkg/processor/processor.go
@@ -230,6 +230,10 @@ func (p *Processor) Run(ctx context.Context) error {
 			if p.acct == nil {
 				return fmt.Errorf("acct: received an accountant event when accountant is not configured")
 			}
+			// SECURITY defense-in-depth: Make sure the accountant did not generate an unexpected message.
+			if !p.acct.IsMessageCoveredByAccountant(k) {
+				return fmt.Errorf("acct: accountant published a message that is not covered by it: `%s`", k.MessageIDString())
+			}
 			p.handleMessage(ctx, k)
 		case v := <-p.injectC:
 			p.handleInjection(ctx, v)