From ca3585a483ca5f6fc4cc54fd1530f89d13e5b7b0 Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Tue, 14 Jul 2020 10:24:43 +0100
Subject: [PATCH] [net/net processing] check banman pointer before
 dereferencing

Although we currently don't do this, it should be possible to create a
CConnman or PeerLogicValidation without a Banman instance. Therefore
always check that banman exists before dereferencing the pointer.

Also add comments to the m_banman members of CConnman and
PeerLogicValidation to document that these may be nullptr.
---
 src/net.cpp            | 4 ++--
 src/net.h              | 1 +
 src/net_processing.cpp | 9 ++++++---
 src/net_processing.h   | 1 +
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 244b0094d..cf5757d6c 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1013,7 +1013,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     SetSocketNoDelay(hSocket);
 
     // Don't accept connections from banned peers.
-    bool banned = m_banman->IsBanned(addr);
+    bool banned = m_banman && m_banman->IsBanned(addr);
     if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && banned)
     {
         LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString());
@@ -1022,7 +1022,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     }
 
     // Only accept connections from discouraged peers if our inbound slots aren't (almost) full.
-    bool discouraged = m_banman->IsDiscouraged(addr);
+    bool discouraged = m_banman && m_banman->IsDiscouraged(addr);
     if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && discouraged)
     {
         LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString());
diff --git a/src/net.h b/src/net.h
index 6673e02d6..b240e52a1 100644
--- a/src/net.h
+++ b/src/net.h
@@ -447,6 +447,7 @@ private:
     std::atomic<int> nBestHeight;
     CClientUIInterface* clientInterface;
     NetEventsInterface* m_msgproc;
+    /** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */
     BanMan* m_banman;
 
     /** SipHasher seeds for deterministic randomness */
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index ad349cf1a..a776fdb21 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2491,8 +2491,10 @@ void ProcessMessage(
             if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60)
                 addr.nTime = nNow - 5 * 24 * 60 * 60;
             pfrom.AddAddressKnown(addr);
-            if (banman->IsDiscouraged(addr)) continue; // Do not process banned/discouraged addresses beyond remembering we received them
-            if (banman->IsBanned(addr)) continue;
+            if (banman && (banman->IsDiscouraged(addr) || banman->IsBanned(addr))) {
+                // Do not process banned/discouraged addresses beyond remembering we received them
+                continue;
+            }
             bool fReachable = IsReachable(addr);
             if (addr.nTime > nSince && !pfrom.fGetAddr && vAddr.size() <= 10 && addr.IsRoutable())
             {
@@ -3346,7 +3348,8 @@ void ProcessMessage(
         std::vector<CAddress> vAddr = connman->GetAddresses();
         FastRandomContext insecure_rand;
         for (const CAddress &addr : vAddr) {
-            if (!banman->IsDiscouraged(addr) && !banman->IsBanned(addr)) {
+            bool banned_or_discouraged = banman && (banman->IsDiscouraged(addr) || banman->IsBanned(addr));
+            if (!banned_or_discouraged) {
                 pfrom.PushAddress(addr, insecure_rand);
             }
         }
diff --git a/src/net_processing.h b/src/net_processing.h
index 3479ef686..fa1555fbe 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -29,6 +29,7 @@ static const int DISCOURAGEMENT_THRESHOLD{100};
 class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface {
 private:
     CConnman* const connman;
+    /** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */
     BanMan* const m_banman;
     ChainstateManager& m_chainman;
     CTxMemPool& m_mempool;