Merge ~ahasenack/ubuntu/+source/net-snmp:focal-snmp-double-free-2012926 into ubuntu/+source/net-snmp:ubuntu/focal-devel

[Andreas Hasenack]

Multiple double free fixes for net-snmp in focal.

The easiest and most reliable way to reproduce the snmpd crash was to use a custom agentx subagent in python and inject an artificial delay just over the snmpd agentx timeout of 1s.

Testing instructions are in the bug.

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/net-snmp-double-free/

The DEP8 test is trivial/superficial, I ran it once but didn't bother in keeping it up-to-date with my PPA iterations.

[Sergio Durigan Junior (sergiodj)]

Thanks, Andreas.

Package builds fine. dep8 is too superficial to be considered useful here, as you said. I verified each patch, did some investigation to make sure that they are minimally correct, and everything seems OK. As you said, there are other commits fixing double frees in the repository, but they're unrelated to agentx.

A small comment about the PPA: you did not enable all architectures there. I really think it's useful to at least make sure nothing strange happens when building on other arches, especially if it's an SRU like this one.

So, in a nutshell: LGTM assuming that the package builds fine on all supported architectures. +1

review: Approve

[Andreas Hasenack (ahasenack)]

Agreed wrt ppa, enabled the remaining arches and triggered a rebuild.

[git-ubuntu bot (git-ubuntu-bot)]

Approvers: ahasenack, sergiodj
Uploaders: ahasenack, sergiodj
MP auto-approved

review: Approve

[Andreas Hasenack (ahasenack)]

Builds in other arches are green, (superficial) tests are also green. Uploading:

Uploading net-snmp_5.8+dfsg-2ubuntu2.8.dsc
Uploading net-snmp_5.8+dfsg-2ubuntu2.8.debian.tar.xz
Uploading net-snmp_5.8+dfsg-2ubuntu2.8_source.buildinfo
Uploading net-snmp_5.8+dfsg-2ubuntu2.8_source.changes

diff --git a/debian/changelog b/debian/changelog
index f02c42c..8640dcd 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,17 @@
+net-snmp (5.8+dfsg-2ubuntu2.8) focal; urgency=medium
+
+  * Fix multiple double free() bugs (LP: #2012926):
+    - d/p/double-free-agentx_got_response.patch: fix double free of
+      delegated cache in agentx_got_response()
+    - d/p/double-free-failed-transport.patch: fix use-after-free in
+      agentx when the transport fails
+    - d/p/double-free-delegated-cache.patch: fix double free in the
+      delegated cache
+    - d/p/double-free-when-NETSNMP_CALLBACK_OP_RESEND-is-set.patch: fix
+      double free when NETSNMP_CALLBACK_OP_RESEND is set to the callback
+
+ -- Andreas Hasenack <[e-mail]>  Fri, 16 Jun 2023 17:04:58 -0300
+
 net-snmp (5.8+dfsg-2ubuntu2.7) focal; urgency=medium
 
   * Fix snmptrapd reconnection issue after hitting MySQL wait_timeout
diff --git a/debian/patches/double-free-agentx_got_response.patch b/debian/patches/double-free-agentx_got_response.patch
new file mode 100644
index 0000000..4ec5005
--- /dev/null
+++ b/debian/patches/double-free-agentx_got_response.patch
@@ -0,0 +1,36 @@
+commit 33f8e2e1d63ad5932ade885ffbff379cab1e50bd
+Author: Shogo Matsumoto <[e-mail]>
+Date:   Tue May 7 09:41:13 2019 +0900
+
+    agent/mibgroup/agentx: Fix double free of delegated cache in agentx_got_response()
+    
+    If snmpd receives a response from subagent immediately after the agentx
+    session closed, agentx_got_response() frees delegated cache twice.
+    
+    This patch changes return value of "response too late on session"
+    in agentx_got_response function to make the caller of this function
+    remove the request in order to prevent double free.
+    
+    See also https://sourceforge.net/p/net-snmp/patches/1392/.
+    
+    Signed-off-by: Shogo Matsumoto <[e-mail]>
+    [ bvanassche: added patch tracker URL ]
+
+Origin: upstream, https://github.com/net-snmp/net-snmp/commit/33f8e2e1d63ad5932ade885ffbff379cab1e50bd
+Bug: https://sourceforge.net/p/net-snmp/bugs/2943/
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/2012926
+Applied-Upstream: v5.8.1.pre1
+Last-Update: 2023-06-16
+diff --git a/agent/mibgroup/agentx/master.c b/agent/mibgroup/agentx/master.c
+index b7a51dfef1..93d562122c 100644
+--- a/agent/mibgroup/agentx/master.c
++++ b/agent/mibgroup/agentx/master.c
+@@ -221,7 +221,7 @@ agentx_got_response(int operation,
+         /* response is too late, free the cache */
+         if (magic)
+             netsnmp_free_delegated_cache((netsnmp_delegated_cache*) magic);
+-        return 0;
++        return 1;
+     }
+     requests = cache->requests;
+ 
diff --git a/debian/patches/double-free-delegated-cache.patch b/debian/patches/double-free-delegated-cache.patch
new file mode 100644
index 0000000..00595a7
--- /dev/null
+++ b/debian/patches/double-free-delegated-cache.patch
@@ -0,0 +1,33 @@
+commit f3e80746fde826cf4665fb959bda78cce061c883
+Author: Bart Van Assche <[e-mail]>
+Date:   Mon Aug 5 20:15:37 2019 -0700
+
+    AgentX: Fix a use-after-free
+    
+    When calling snmp_async_send(), the callback function is called whether or
+    not sending succeeds. Since the agentx_got_response() callback function
+    frees the cache unconditionally, calling netsnmp_free_delegated_cache()
+    explicitly after snmp_async_send() is wrong.
+    
+    See also https://sourceforge.net/p/net-snmp/bugs/2943/.
+    
+    Fixes: f9304c83f762 ("CHANGES: PATCH 1633670: fixed snmpd crashing when an AgentX subagent disconnect in the middle of processing of a request.") # v5.8.
+
+Origin: upstream, https://github.com/net-snmp/net-snmp/commit/f3e80746fde826cf4665fb959bda78cce061c883
+Bug: https://sourceforge.net/p/net-snmp/bugs/2943/
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/2012926
+Applied-Upstream: v5.8.1.pre1
+Last-Update: 2023-06-16
+diff --git a/agent/mibgroup/agentx/master.c b/agent/mibgroup/agentx/master.c
+index e3ff8aecb3..a9c29da783 100644
+--- a/agent/mibgroup/agentx/master.c
++++ b/agent/mibgroup/agentx/master.c
+@@ -616,8 +616,6 @@ agentx_master_handler(netsnmp_mib_handler *handler,
+     result = snmp_async_send(ax_session, pdu, agentx_got_response, cb_data);
+     if (result == 0) {
+         snmp_free_pdu(pdu);
+-        if (cb_data)
+-            netsnmp_free_delegated_cache((netsnmp_delegated_cache*) cb_data);
+     }
+ 
+     return SNMP_ERR_NOERROR;
diff --git a/debian/patches/double-free-failed-transport.patch b/debian/patches/double-free-failed-transport.patch
new file mode 100644
index 0000000..0804963
--- /dev/null
+++ b/debian/patches/double-free-failed-transport.patch
@@ -0,0 +1,40 @@
+commit d73ebccfe45be49f70ab0a3f5dd298db78c99f6d
+Author: Bart Van Assche <[e-mail]>
+Date:   Thu May 16 13:48:03 2019 +0200
+
+    AgentX: Fix two use-after-free issues
+    
+    snmp_add_full() frees the transport if it fails. This was detected by Coverity.
+
+Origin: upstream, https://github.com/net-snmp/net-snmp/commit/d73ebccfe45be49f70ab0a3f5dd298db78c99f6d
+Bug: https://sourceforge.net/p/net-snmp/bugs/2943/
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/2012926
+Applied-Upstream: v5.8.1.pre1
+Last-Update: 2023-06-16
+diff --git a/agent/mibgroup/agentx/master.c b/agent/mibgroup/agentx/master.c
+index 93d562122c..e3ff8aecb3 100644
+--- a/agent/mibgroup/agentx/master.c
++++ b/agent/mibgroup/agentx/master.c
+@@ -185,6 +185,9 @@ real_init_master(void)
+             session =
+                 snmp_add_full(&sess, t, NULL, agentx_parse, NULL, NULL,
+                               agentx_realloc_build, agentx_check_packet, NULL);
++            /* snmp_add_full() frees 't' upon failure. */
++            if (!session)
++                t = NULL;
+         }
+         if (session == NULL) {
+             netsnmp_transport_free(t);
+diff --git a/agent/mibgroup/agentx/subagent.c b/agent/mibgroup/agentx/subagent.c
+index cf0bf62cf7..c11f56dd46 100644
+--- a/agent/mibgroup/agentx/subagent.c
++++ b/agent/mibgroup/agentx/subagent.c
+@@ -870,6 +870,8 @@ subagent_open_master_session(void)
+                       agentx_realloc_build, agentx_check_packet, NULL);
+ 
+     if (main_session == NULL) {
++        /* snmp_add_full() frees 't' upon failure. */
++        t = NULL;
+         if (!netsnmp_ds_get_boolean(NETSNMP_DS_APPLICATION_ID,
+                                     NETSNMP_DS_AGENT_NO_CONNECTION_WARNINGS)) {
+             char buf[1024];
diff --git a/debian/patches/double-free-when-NETSNMP_CALLBACK_OP_RESEND-is-set.patch b/debian/patches/double-free-when-NETSNMP_CALLBACK_OP_RESEND-is-set.patch
new file mode 100644
index 0000000..212ad26
--- /dev/null
+++ b/debian/patches/double-free-when-NETSNMP_CALLBACK_OP_RESEND-is-set.patch
@@ -0,0 +1,40 @@
+commit 7f88b06bf0f4140bc03cabacd35eaeffdf21d722
+Author: Anders Wallin <[e-mail]>
+Date:   Sun Apr 7 18:31:16 2019 -0400
+
+    agentx/master: Return when NETSNMP_CALLBACK_OP_RESEND is set to the callback
+    
+    snmpd is terminated abnormally due to the double free for the
+    request cache after the request is resend.
+    
+    That is because the callback for NETSNMP_CALLBACK_OP_RESEND isn't
+    cared and the cache is freed wrongly.
+    
+    Let's just return if NETSNMP_CALLBACK_OP_RESEND is set on the
+    callback.
+    
+    Fixes: b7b50bbac ("snmp_send callback updates")
+    
+    Signed-off-by: Anders Wallin <[e-mail]>
+
+Origin: upstream, https://github.com/net-snmp/net-snmp/commit/7f88b06bf0f4140bc03cabacd35eaeffdf21d722
+Bug: https://sourceforge.net/p/net-snmp/bugs/2943/
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/2012926
+Applied-Upstream: v5.8.1.pre1
+Last-Update: 2023-06-16
+diff --git a/agent/mibgroup/agentx/master.c b/agent/mibgroup/agentx/master.c
+index 99c4123ed3..b7a51dfef1 100644
+--- a/agent/mibgroup/agentx/master.c
++++ b/agent/mibgroup/agentx/master.c
+@@ -280,6 +280,11 @@ agentx_got_response(int operation,
+         netsnmp_free_delegated_cache(cache);
+         return 0;
+ 
++    case NETSNMP_CALLBACK_OP_RESEND:
++        DEBUGMSGTL(("agentx/master", "resend on session %8p req=0x%x\n",
++                    session, (unsigned)reqid));
++        return 0;
++
+     case NETSNMP_CALLBACK_OP_RECEIVED_MESSAGE:
+         /*
+          * This session is alive 
diff --git a/debian/patches/series b/debian/patches/series
index f4992f8..80ccd38 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -53,3 +53,7 @@ CVE-2022-4479x-2.patch
 CVE-2022-4479x-3.patch
 snmptrapd-mysql-reconnection-after-hitting-wait_timeout.patch
 snmptrapd-mysql-fix-build-error.patch
+double-free-agentx_got_response.patch
+double-free-failed-transport.patch
+double-free-delegated-cache.patch
+double-free-when-NETSNMP_CALLBACK_OP_RESEND-is-set.patch