diff --git a/debian/changelog b/debian/changelog index d6d30a8..b77191d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,24 @@ +net-snmp (5.8+dfsg-2ubuntu3) groovy; urgency=medium + + * Fix segmentation fault that happens when using the snmpv3 protocol + with snmpbulkget. (LP: #1877027) + - d/p/move-securityStateRef-into-free_securityStateRef.patch: + Consolidate the check of the securityStateRef pointer into the + free_securityStateRef function. + - d/p/prevent-snmpv3-bulkget-errors-double-free.patch: + Prevent snmpv3 bulkget errors from becoming resulting in a + double free. + - d/p/fix-usmStateReference-free.patch: + Fix typo on usm_free_usmStateReference from last patch. + - d/p/unexport-struct-usmStateReference.patch: + Unexport struct usmStateReference and to prevent ABI breakages, + since it will be necessary to add a reference count to it. + - d/p/introduce-refcount-usmStateReference.patch: + Introduce refcount in the struct usmStateReference, and adjust + code to properly use the field. + + -- Sergio Durigan Junior Tue, 23 Jun 2020 14:37:42 -0400 + net-snmp (5.8+dfsg-2ubuntu2) focal; urgency=medium * d/p/lp1871307-log-once-proc-net-if_inet6-failure.patch (LP: #1871307): diff --git a/debian/patches/fix-usmStateReference-free.patch b/debian/patches/fix-usmStateReference-free.patch new file mode 100644 index 0000000..0fe4602 --- /dev/null +++ b/debian/patches/fix-usmStateReference-free.patch @@ -0,0 +1,31 @@ +From: Ming Chen +Date: Wed, 5 Jun 2019 19:58:44 -0700 +Subject: libsnmp: Fix usm_free_usmStateReference() + +See also https://sourceforge.net/p/net-snmp/bugs/2923/. + +Fixes: adc9b71aba91 ("snmpd: Avoid that snmpv3 bulkget errors result in a double free") + +Author: Ming Chen +Origin: upstream, https://github.com/net-snmp/net-snmp/commit/7384a8b55 +Last-Update: 2020-06-17 +Bug: https://sourceforge.net/p/net-snmp/bugs/2923/ +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/1877027 +Reviewed-By: Sergio Durigan Junior +--- + snmplib/snmpusm.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/snmplib/snmpusm.c b/snmplib/snmpusm.c +index c1a6dcc..0853e8d 100644 +--- a/snmplib/snmpusm.c ++++ b/snmplib/snmpusm.c +@@ -305,7 +305,7 @@ usm_free_usmStateReference(void *old) + SNMP_FREE(old_ref->usr_engine_id); + if (old_ref->usr_auth_protocol_length) + SNMP_FREE(old_ref->usr_auth_protocol); +- if (old_ref->usr_auth_protocol_length) ++ if (old_ref->usr_priv_protocol_length) + SNMP_FREE(old_ref->usr_priv_protocol); + + if (old_ref->usr_auth_key_length && old_ref->usr_auth_key) { diff --git a/debian/patches/introduce-refcount-usmStateReference.patch b/debian/patches/introduce-refcount-usmStateReference.patch new file mode 100644 index 0000000..8f6fb30 --- /dev/null +++ b/debian/patches/introduce-refcount-usmStateReference.patch @@ -0,0 +1,169 @@ +From: Bart Van Assche +Date: Sat, 27 Jul 2019 19:34:09 -0700 +Subject: libsnmp, + USM: Introduce a reference count in struct usmStateReference + +This patch fixes https://sourceforge.net/p/net-snmp/bugs/2956/. + +Author: Bart Van Assche +Origin: backport, https://github.com/net-snmp/net-snmp/commit/5f881d3bf +Last-Update: 2020-06-17 +Bug: https://sourceforge.net/p/net-snmp/bugs/2956/ +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/1877027 +Reviewed-By: Sergio Durigan Junior +--- + snmplib/snmp_client.c | 21 ++++----------- + snmplib/snmpusm.c | 73 +++++++++++++++++++++++++++++++++------------------ + 2 files changed, 52 insertions(+), 42 deletions(-) + +diff --git a/snmplib/snmp_client.c b/snmplib/snmp_client.c +index 2a46351..b2ea891 100644 +--- a/snmplib/snmp_client.c ++++ b/snmplib/snmp_client.c +@@ -402,27 +402,16 @@ _clone_pdu_header(netsnmp_pdu *pdu) + return NULL; + } + +- if (pdu->securityStateRef && +- pdu->command == SNMP_MSG_TRAP2) { +- +- ret = usm_clone_usmStateReference((struct usmStateReference *) pdu->securityStateRef, +- (struct usmStateReference **) &newpdu->securityStateRef ); +- +- if (ret) +- { ++ sptr = find_sec_mod(newpdu->securityModel); ++ if (sptr && sptr->pdu_clone) { ++ /* call security model if it needs to know about this */ ++ ret = sptr->pdu_clone(pdu, newpdu); ++ if (ret) { + snmp_free_pdu(newpdu); + return NULL; + } + } + +- if ((sptr = find_sec_mod(newpdu->securityModel)) != NULL && +- sptr->pdu_clone != NULL) { +- /* +- * call security model if it needs to know about this +- */ +- (*sptr->pdu_clone) (pdu, newpdu); +- } +- + return newpdu; + } + +diff --git a/snmplib/snmpusm.c b/snmplib/snmpusm.c +index ec2074c..ba4ca5f 100644 +--- a/snmplib/snmpusm.c ++++ b/snmplib/snmpusm.c +@@ -85,6 +85,7 @@ netsnmp_feature_child_of(usm_support, usm_all) + netsnmp_feature_require(usm_support) + + struct usmStateReference { ++ int refcnt; + char *usr_name; + size_t usr_name_length; + u_char *usr_engine_id; +@@ -301,43 +302,63 @@ free_enginetime_on_shutdown(int majorid, int minorid, void *serverarg, + struct usmStateReference * + usm_malloc_usmStateReference(void) + { +- struct usmStateReference *retval = (struct usmStateReference *) +- calloc(1, sizeof(struct usmStateReference)); ++ struct usmStateReference *retval; ++ ++ retval = calloc(1, sizeof(struct usmStateReference)); ++ if (retval) ++ retval->refcnt = 1; + + return retval; + } /* end usm_malloc_usmStateReference() */ + ++static int ++usm_clone(netsnmp_pdu *pdu, netsnmp_pdu *new_pdu) ++{ ++ struct usmStateReference *ref = pdu->securityStateRef; ++ struct usmStateReference **new_ref = ++ (struct usmStateReference **)&new_pdu->securityStateRef; ++ int ret = 0; ++ ++ if (!ref) ++ return ret; ++ ++ if (pdu->command == SNMP_MSG_TRAP2) { ++ netsnmp_assert(pdu->securityModel == SNMP_DEFAULT_SECMODEL); ++ ret = usm_clone_usmStateReference(ref, new_ref); ++ } else { ++ netsnmp_assert(ref == *new_ref); ++ ref->refcnt++; ++ } ++ ++ return ret; ++} + + void + usm_free_usmStateReference(void *old) + { +- struct usmStateReference *old_ref = (struct usmStateReference *) old; +- +- if (old_ref) { +- +- if (old_ref->usr_name_length) +- SNMP_FREE(old_ref->usr_name); +- if (old_ref->usr_engine_id_length) +- SNMP_FREE(old_ref->usr_engine_id); +- if (old_ref->usr_auth_protocol_length) +- SNMP_FREE(old_ref->usr_auth_protocol); +- if (old_ref->usr_priv_protocol_length) +- SNMP_FREE(old_ref->usr_priv_protocol); +- +- if (old_ref->usr_auth_key_length && old_ref->usr_auth_key) { +- SNMP_ZERO(old_ref->usr_auth_key, old_ref->usr_auth_key_length); +- SNMP_FREE(old_ref->usr_auth_key); +- } +- if (old_ref->usr_priv_key_length && old_ref->usr_priv_key) { +- SNMP_ZERO(old_ref->usr_priv_key, old_ref->usr_priv_key_length); +- SNMP_FREE(old_ref->usr_priv_key); +- } ++ struct usmStateReference *ref = old; ++ ++ if (!ref) ++ return; + +- SNMP_ZERO(old_ref, sizeof(*old_ref)); +- SNMP_FREE(old_ref); ++ if (--ref->refcnt > 0) ++ return; ++ ++ SNMP_FREE(ref->usr_name); ++ SNMP_FREE(ref->usr_engine_id); ++ SNMP_FREE(ref->usr_auth_protocol); ++ SNMP_FREE(ref->usr_priv_protocol); + ++ if (ref->usr_auth_key_length && ref->usr_auth_key) { ++ SNMP_ZERO(ref->usr_auth_key, ref->usr_auth_key_length); ++ SNMP_FREE(ref->usr_auth_key); ++ } ++ if (ref->usr_priv_key_length && ref->usr_priv_key) { ++ SNMP_ZERO(ref->usr_priv_key, ref->usr_priv_key_length); ++ SNMP_FREE(ref->usr_priv_key); + } + ++ SNMP_FREE(ref); + } /* end usm_free_usmStateReference() */ + + struct usmUser * +@@ -3337,6 +3358,7 @@ init_usm(void) + def->encode_reverse = usm_secmod_rgenerate_out_msg; + def->encode_forward = usm_secmod_generate_out_msg; + def->decode = usm_secmod_process_in_msg; ++ def->pdu_clone = usm_clone; + def->pdu_free_state_ref = usm_free_usmStateReference; + def->session_setup = usm_session_init; + def->handle_report = usm_handle_report; +@@ -5110,4 +5132,3 @@ get_default_privtype(size_t * len) + *len = defaultPrivTypeLen; + return defaultPrivType; + } +- diff --git a/debian/patches/move-free-securityStateRef-into-snmp-free-pdu.patch b/debian/patches/move-free-securityStateRef-into-snmp-free-pdu.patch new file mode 100644 index 0000000..a568095 --- /dev/null +++ b/debian/patches/move-free-securityStateRef-into-snmp-free-pdu.patch @@ -0,0 +1,92 @@ +From: Bart Van Assche +Date: Tue, 13 Aug 2019 20:54:23 -0700 +Subject: libsnmp: Move the free_securityStateRef() call into snmp_free_pdu() + +This patch fixes a memory leak that was introduced in commit 5f881d3bf245 +("libsnmp, USM: Introduce a reference count in struct usmStateReference"). + +This patch partially reverts commit adc9b71aba91 ("snmpd: Avoid that snmpv3 +bulkget errors result in a double free"). + +See also https://sourceforge.net/p/net-snmp/bugs/2938/. + +Author: Bart Van Assche +Origin: backport, https://github.com/net-snmp/net-snmp/commit/87bd90d04 +Last-Update: 2020-06-17 +Bug: https://sourceforge.net/p/net-snmp/bugs/2938/ +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/1877027 +Reviewed-By: Sergio Durigan Junior +--- + include/net-snmp/pdu_api.h | 2 -- + snmplib/snmp_api.c | 24 +++--------------------- + 2 files changed, 3 insertions(+), 23 deletions(-) + +diff --git a/include/net-snmp/pdu_api.h b/include/net-snmp/pdu_api.h +index 270aff0..125595d 100644 +--- a/include/net-snmp/pdu_api.h ++++ b/include/net-snmp/pdu_api.h +@@ -19,8 +19,6 @@ NETSNMP_IMPORT + netsnmp_pdu *snmp_fix_pdu( netsnmp_pdu *pdu, int idx); + NETSNMP_IMPORT + void snmp_free_pdu( netsnmp_pdu *pdu); +-NETSNMP_IMPORT +-void snmp_free_securityStateRef( netsnmp_pdu *pdu); + + #ifdef __cplusplus + } +diff --git a/snmplib/snmp_api.c b/snmplib/snmp_api.c +index 046b247..0fea19e 100644 +--- a/snmplib/snmp_api.c ++++ b/snmplib/snmp_api.c +@@ -4033,17 +4033,6 @@ free_securityStateRef(netsnmp_pdu* pdu) + pdu->securityStateRef = NULL; + } + +-/* +- * This function is here to provide a separate call to +- * free the securityStateRef memory. This is needed to prevent +- * a double free if this memory is freed in snmp_free_pdu. +- */ +-void +-snmp_free_securityStateRef(netsnmp_pdu* pdu) +-{ +- free_securityStateRef(pdu); +-} +- + #define ERROR_STAT_LENGTH 11 + + int +@@ -5493,6 +5482,9 @@ snmp_free_pdu(netsnmp_pdu *pdu) + return; + } + */ ++ ++ free_securityStateRef(pdu); ++ + if ((sptr = find_sec_mod(pdu->securityModel)) != NULL && + sptr->pdu_free != NULL) { + (*sptr->pdu_free) (pdu); +@@ -5647,10 +5639,6 @@ _sess_process_packet_parse_pdu(void *sessp, netsnmp_session * sp, + } + + if (ret != SNMP_ERR_NOERROR) { +- /* +- * Call the security model to free any securityStateRef supplied w/ msg. +- */ +- free_securityStateRef(pdu); + snmp_free_pdu(pdu); + return NULL; + } +@@ -5822,12 +5810,6 @@ _sess_process_packet_handle_pdu(void *sessp, netsnmp_session * sp, + } + } + +- /* +- * Call USM to free any securityStateRef supplied with the message. +- */ +- if (pdu->command == SNMP_MSG_TRAP2) +- free_securityStateRef(pdu); +- + if (!handled) { + if (sp->flags & SNMP_FLAGS_SHARED_SOCKET) + return -2; diff --git a/debian/patches/move-securityStateRef-into-free_securityStateRef.patch b/debian/patches/move-securityStateRef-into-free_securityStateRef.patch new file mode 100644 index 0000000..fe6697c --- /dev/null +++ b/debian/patches/move-securityStateRef-into-free_securityStateRef.patch @@ -0,0 +1,78 @@ +From: Bart Van Assche +Date: Sat, 25 May 2019 16:33:31 +0200 +Subject: libsnmp: Move the securityStateRef check into + free_securityStateRef() + +Instead of making each free_securityStateRef() caller check the +securityStateRef pointer, move that check into free_securityStateRef(). + +Author: Bart Van Assche +Origin: backport, https://github.com/net-snmp/net-snmp/commit/92ccd5a82 +Last-Update: 2020-06-17 +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/1877027 +Reviewed-By: Sergio Durigan Junior +--- + agent/snmp_agent.c | 1 + + snmplib/snmp_api.c | 21 ++++++++++----------- + 2 files changed, 11 insertions(+), 11 deletions(-) + +diff --git a/snmplib/snmp_api.c b/snmplib/snmp_api.c +index 6754fa6..bfc052b 100644 +--- a/snmplib/snmp_api.c ++++ b/snmplib/snmp_api.c +@@ -4011,7 +4011,12 @@ snmpv3_parse(netsnmp_pdu *pdu, + static void + free_securityStateRef(netsnmp_pdu* pdu) + { +- struct snmp_secmod_def *sptr = find_sec_mod(pdu->securityModel); ++ struct snmp_secmod_def *sptr; ++ ++ if (!pdu->securityStateRef) ++ return; ++ ++ sptr = find_sec_mod(pdu->securityModel); + if (sptr) { + if (sptr->pdu_free_state_ref) { + (*sptr->pdu_free_state_ref) (pdu->securityStateRef); +@@ -4122,9 +4127,7 @@ snmpv3_make_report(netsnmp_pdu *pdu, int error) + * FIX - yes they should but USM needs to follow new EoP to determine + * which cached values to use + */ +- if (pdu->securityStateRef) { +- free_securityStateRef(pdu); +- } ++ free_securityStateRef(pdu); + + if (error == SNMPERR_USM_NOTINTIMEWINDOW) { + pdu->securityLevel = SNMP_SEC_LEVEL_AUTHNOPRIV; +@@ -5636,9 +5639,7 @@ _sess_process_packet_parse_pdu(void *sessp, netsnmp_session * sp, + /* + * Call the security model to free any securityStateRef supplied w/ msg. + */ +- if (pdu->securityStateRef != NULL) { +- free_securityStateRef(pdu); +- } ++ free_securityStateRef(pdu); + snmp_free_pdu(pdu); + return NULL; + } +@@ -5662,9 +5663,7 @@ _sess_process_packet_handle_pdu(void *sessp, netsnmp_session * sp, + /* + * Call USM to free any securityStateRef supplied with the message. + */ +- if (pdu->securityStateRef) { +- free_securityStateRef(pdu); +- } ++ free_securityStateRef(pdu); + + for (rp = isp->requests; rp; orp = rp, rp = rp->next_request) { + snmp_callback callback; +@@ -5815,7 +5814,7 @@ _sess_process_packet_handle_pdu(void *sessp, netsnmp_session * sp, + /* + * Call USM to free any securityStateRef supplied with the message. + */ +- if (pdu->securityStateRef && pdu->command == SNMP_MSG_TRAP2) ++ if (pdu->command == SNMP_MSG_TRAP2) + free_securityStateRef(pdu); + + if (!handled) { diff --git a/debian/patches/prevent-snmpv3-bulkget-errors-double-free.patch b/debian/patches/prevent-snmpv3-bulkget-errors-double-free.patch new file mode 100644 index 0000000..61b8348 --- /dev/null +++ b/debian/patches/prevent-snmpv3-bulkget-errors-double-free.patch @@ -0,0 +1,349 @@ +From: Sam Tannous +Date: Wed, 10 Apr 2019 06:57:21 -0700 +Subject: snmpd: Avoid that snmpv3 bulkget errors result in a double free + +See also https://sourceforge.net/p/net-snmp/bugs/2923/. +See also https://sourceforge.net/p/net-snmp/patches/1388/. + +Author: Ming Chen +Origin: upstream, https://github.com/net-snmp/net-snmp/commit/adc9b71ab +Last-Update: 2020-06-17 +Bug: https://sourceforge.net/p/net-snmp/bugs/2923/ +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/1877027 +Reviewed-By: Sergio Durigan Junior +--- + include/net-snmp/pdu_api.h | 2 ++ + snmplib/snmp_api.c | 11 ++++++++++ + snmplib/snmpusm.c | 51 ++++++++++------------------------------------ + 3 files changed, 24 insertions(+), 40 deletions(-) + +diff --git a/include/net-snmp/pdu_api.h b/include/net-snmp/pdu_api.h +index 125595d..270aff0 100644 +--- a/include/net-snmp/pdu_api.h ++++ b/include/net-snmp/pdu_api.h +@@ -19,6 +19,8 @@ NETSNMP_IMPORT + netsnmp_pdu *snmp_fix_pdu( netsnmp_pdu *pdu, int idx); + NETSNMP_IMPORT + void snmp_free_pdu( netsnmp_pdu *pdu); ++NETSNMP_IMPORT ++void snmp_free_securityStateRef( netsnmp_pdu *pdu); + + #ifdef __cplusplus + } +diff --git a/snmplib/snmp_api.c b/snmplib/snmp_api.c +index bfc052b..046b247 100644 +--- a/snmplib/snmp_api.c ++++ b/snmplib/snmp_api.c +@@ -4033,6 +4033,17 @@ free_securityStateRef(netsnmp_pdu* pdu) + pdu->securityStateRef = NULL; + } + ++/* ++ * This function is here to provide a separate call to ++ * free the securityStateRef memory. This is needed to prevent ++ * a double free if this memory is freed in snmp_free_pdu. ++ */ ++void ++snmp_free_securityStateRef(netsnmp_pdu* pdu) ++{ ++ free_securityStateRef(pdu); ++} ++ + #define ERROR_STAT_LENGTH 11 + + int +diff --git a/snmplib/snmpusm.c b/snmplib/snmpusm.c +index e697c23..c1a6dcc 100644 +--- a/snmplib/snmpusm.c ++++ b/snmplib/snmpusm.c +@@ -299,16 +299,20 @@ usm_free_usmStateReference(void *old) + + if (old_ref) { + +- SNMP_FREE(old_ref->usr_name); +- SNMP_FREE(old_ref->usr_engine_id); +- SNMP_FREE(old_ref->usr_auth_protocol); +- SNMP_FREE(old_ref->usr_priv_protocol); +- +- if (old_ref->usr_auth_key) { ++ if (old_ref->usr_name_length) ++ SNMP_FREE(old_ref->usr_name); ++ if (old_ref->usr_engine_id_length) ++ SNMP_FREE(old_ref->usr_engine_id); ++ if (old_ref->usr_auth_protocol_length) ++ SNMP_FREE(old_ref->usr_auth_protocol); ++ if (old_ref->usr_auth_protocol_length) ++ SNMP_FREE(old_ref->usr_priv_protocol); ++ ++ if (old_ref->usr_auth_key_length && old_ref->usr_auth_key) { + SNMP_ZERO(old_ref->usr_auth_key, old_ref->usr_auth_key_length); + SNMP_FREE(old_ref->usr_auth_key); + } +- if (old_ref->usr_priv_key) { ++ if (old_ref->usr_priv_key_length && old_ref->usr_priv_key) { + SNMP_ZERO(old_ref->usr_priv_key, old_ref->usr_priv_key_length); + SNMP_FREE(old_ref->usr_priv_key); + } +@@ -1039,7 +1043,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + if ((user = usm_get_user(secEngineID, secEngineIDLen, secName)) + == NULL && secLevel != SNMP_SEC_LEVEL_NOAUTH) { + DEBUGMSGTL(("usm", "Unknown User(%s)\n", secName)); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_UNKNOWNSECURITYNAME; + } + +@@ -1091,7 +1094,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + thePrivProtocolLength) == 1) { + DEBUGMSGTL(("usm", "Unsupported Security Level (%d)\n", + theSecLevel)); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_UNSUPPORTEDSECURITYLEVEL; + } + +@@ -1121,7 +1123,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + &msgAuthParmLen, &msgPrivParmLen, &otstlen, + &seq_len, &msgSecParmLen) == -1) { + DEBUGMSGTL(("usm", "Failed calculating offsets.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_GENERICERROR; + } + +@@ -1143,7 +1144,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + ptr = *wholeMsg = globalData; + if (theTotalLength > *wholeMsgLen) { + DEBUGMSGTL(("usm", "Message won't fit in buffer.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_GENERICERROR; + } + +@@ -1169,7 +1169,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + htonl(boots_uint), htonl(time_uint), + &ptr[privParamsOffset]) == -1) { + DEBUGMSGTL(("usm", "Can't set AES iv.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_GENERICERROR; + } + } +@@ -1185,7 +1184,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + &ptr[privParamsOffset]) + == -1)) { + DEBUGMSGTL(("usm", "Can't set DES-CBC salt.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_GENERICERROR; + } + } +@@ -1198,7 +1196,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + &ptr[dataOffset], &encrypted_length) + != SNMP_ERR_NOERROR) { + DEBUGMSGTL(("usm", "encryption error.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_ENCRYPTIONERROR; + } + #ifdef NETSNMP_ENABLE_TESTING_CODE +@@ -1226,7 +1223,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + if ((encrypted_length != (theTotalLength - dataOffset)) + || (salt_length != msgPrivParmLen)) { + DEBUGMSGTL(("usm", "encryption length error.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_ENCRYPTIONERROR; + } + +@@ -1362,7 +1358,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + + if (temp_sig == NULL) { + DEBUGMSGTL(("usm", "Out of memory.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_GENERICERROR; + } + +@@ -1376,7 +1371,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + SNMP_ZERO(temp_sig, temp_sig_len); + SNMP_FREE(temp_sig); + DEBUGMSGTL(("usm", "Signing failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_AUTHENTICATIONFAILURE; + } + +@@ -1384,7 +1378,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + SNMP_ZERO(temp_sig, temp_sig_len); + SNMP_FREE(temp_sig); + DEBUGMSGTL(("usm", "Signing lengths failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_AUTHENTICATIONFAILURE; + } + +@@ -1398,7 +1391,6 @@ usm_generate_out_msg(int msgProcModel, /* (UNUSED) */ + /* + * endif -- create keyed hash + */ +- usm_free_usmStateReference(secStateRef); + + DEBUGMSGTL(("usm", "USM processing completed.\n")); + +@@ -1548,7 +1540,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + if ((user = usm_get_user(secEngineID, secEngineIDLen, secName)) + == NULL && secLevel != SNMP_SEC_LEVEL_NOAUTH) { + DEBUGMSGTL(("usm", "Unknown User\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_UNKNOWNSECURITYNAME; + } + +@@ -1601,7 +1592,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + DEBUGMSGTL(("usm", "Unsupported Security Level or type (%d)\n", + theSecLevel)); + +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_UNSUPPORTEDSECURITYLEVEL; + } + +@@ -1636,7 +1626,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + DEBUGMSGTL(("usm", + "couldn't malloc %d bytes for encrypted PDU\n", + (int)ciphertextlen)); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_MALLOC; + } + +@@ -1652,7 +1641,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + htonl(boots_uint), htonl(time_uint), + iv) == -1) { + DEBUGMSGTL(("usm", "Can't set AES iv.\n")); +- usm_free_usmStateReference(secStateRef); + SNMP_FREE(ciphertext); + return SNMPERR_USM_GENERICERROR; + } +@@ -1667,7 +1655,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + thePrivKeyLength - 8, + iv) == -1)) { + DEBUGMSGTL(("usm", "Can't set DES-CBC salt.\n")); +- usm_free_usmStateReference(secStateRef); + SNMP_FREE(ciphertext); + return SNMPERR_USM_GENERICERROR; + } +@@ -1686,7 +1673,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + scopedPdu, scopedPduLen, + ciphertext, &ciphertextlen) != SNMP_ERR_NOERROR) { + DEBUGMSGTL(("usm", "encryption error.\n")); +- usm_free_usmStateReference(secStateRef); + SNMP_FREE(ciphertext); + return SNMPERR_USM_ENCRYPTIONERROR; + } +@@ -1703,7 +1689,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + ciphertext, ciphertextlen); + if (rc == 0) { + DEBUGMSGTL(("usm", "Encryption failed.\n")); +- usm_free_usmStateReference(secStateRef); + SNMP_FREE(ciphertext); + return SNMPERR_USM_ENCRYPTIONERROR; + } +@@ -1743,7 +1728,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + DEBUGINDENTLESS(); + if (rc == 0) { + DEBUGMSGTL(("usm", "building privParams failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1766,7 +1750,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + DEBUGINDENTLESS(); + if (rc == 0) { + DEBUGMSGTL(("usm", "building authParams failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1789,7 +1772,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + DEBUGINDENTLESS(); + if (rc == 0) { + DEBUGMSGTL(("usm", "building authParams failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1805,7 +1787,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + if (rc == 0) { + DEBUGMSGTL(("usm", + "building msgAuthoritativeEngineTime failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1821,7 +1802,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + if (rc == 0) { + DEBUGMSGTL(("usm", + "building msgAuthoritativeEngineBoots failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1833,7 +1813,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + DEBUGINDENTLESS(); + if (rc == 0) { + DEBUGMSGTL(("usm", "building msgAuthoritativeEngineID failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1846,7 +1825,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + *offset - sp_offset); + if (rc == 0) { + DEBUGMSGTL(("usm", "building usm security parameters failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1860,7 +1838,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + + if (rc == 0) { + DEBUGMSGTL(("usm", "building msgSecurityParameters failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1870,7 +1847,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + while ((*wholeMsgLen - *offset) < globalDataLen) { + if (!asn_realloc(wholeMsg, wholeMsgLen)) { + DEBUGMSGTL(("usm", "building global data failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + } +@@ -1886,7 +1862,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + ASN_CONSTRUCTOR), *offset); + if (rc == 0) { + DEBUGMSGTL(("usm", "building master packet sequence failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_TOO_LONG; + } + +@@ -1904,7 +1879,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + + if (temp_sig == NULL) { + DEBUGMSGTL(("usm", "Out of memory.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_GENERICERROR; + } + +@@ -1915,14 +1889,12 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + != SNMP_ERR_NOERROR) { + SNMP_FREE(temp_sig); + DEBUGMSGTL(("usm", "Signing failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_AUTHENTICATIONFAILURE; + } + + if (temp_sig_len != msgAuthParmLen) { + SNMP_FREE(temp_sig); + DEBUGMSGTL(("usm", "Signing lengths failed.\n")); +- usm_free_usmStateReference(secStateRef); + return SNMPERR_USM_AUTHENTICATIONFAILURE; + } + +@@ -1933,7 +1905,6 @@ usm_rgenerate_out_msg(int msgProcModel, /* (UNUSED) */ + /* + * endif -- create keyed hash + */ +- usm_free_usmStateReference(secStateRef); + DEBUGMSGTL(("usm", "USM processing completed.\n")); + return SNMPERR_SUCCESS; + } /* end usm_rgenerate_out_msg() */ diff --git a/debian/patches/series b/debian/patches/series index 8160545..eac6e7a 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -37,3 +37,9 @@ autofs-skip-autofs-entries.patch autofs-fix-a-recently-introduced-bug.patch fix-check-hr-filesys-autofs.patch lp1871307-log-once-proc-net-if_inet6-failure.patch +move-securityStateRef-into-free_securityStateRef.patch +prevent-snmpv3-bulkget-errors-double-free.patch +fix-usmStateReference-free.patch +unexport-struct-usmStateReference.patch +introduce-refcount-usmStateReference.patch +move-free-securityStateRef-into-snmp-free-pdu.patch diff --git a/debian/patches/unexport-struct-usmStateReference.patch b/debian/patches/unexport-struct-usmStateReference.patch new file mode 100644 index 0000000..aeb3081 --- /dev/null +++ b/debian/patches/unexport-struct-usmStateReference.patch @@ -0,0 +1,74 @@ +From: Bart Van Assche +Date: Tue, 23 Jul 2019 10:52:28 -0700 +Subject: libsnmp: Unexport struct usmStateReference + +Certain snmpd crashes can only be fixed by introducing a reference +count in struct usmStateReference. Unexport that structure such that +changing it does not affect the ABI. + +Author: Bart Van Assche +Origin: upstream, https://github.com/net-snmp/net-snmp/commit/39381c4d2 +Last-Update: 2020-06-17 +Bug: https://sourceforge.net/p/net-snmp/bugs/2956/ +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/1877027 +Reviewed-By: Sergio Durigan Junior +--- + include/net-snmp/library/snmpusm.h | 17 +---------------- + snmplib/snmpusm.c | 16 ++++++++++++++++ + 2 files changed, 17 insertions(+), 16 deletions(-) + +diff --git a/include/net-snmp/library/snmpusm.h b/include/net-snmp/library/snmpusm.h +index 3f60787..49061d8 100644 +--- a/include/net-snmp/library/snmpusm.h ++++ b/include/net-snmp/library/snmpusm.h +@@ -42,22 +42,7 @@ extern "C" { + /* + * Structures. + */ +- struct usmStateReference { +- char *usr_name; +- size_t usr_name_length; +- u_char *usr_engine_id; +- size_t usr_engine_id_length; +- oid *usr_auth_protocol; +- size_t usr_auth_protocol_length; +- u_char *usr_auth_key; +- size_t usr_auth_key_length; +- oid *usr_priv_protocol; +- size_t usr_priv_protocol_length; +- u_char *usr_priv_key; +- size_t usr_priv_key_length; +- u_int usr_sec_level; +- }; +- ++ struct usmStateReference; + + /* + * struct usmUser: a structure to represent a given user in a list +diff --git a/snmplib/snmpusm.c b/snmplib/snmpusm.c +index 0853e8d..ec2074c 100644 +--- a/snmplib/snmpusm.c ++++ b/snmplib/snmpusm.c +@@ -84,6 +84,22 @@ netsnmp_feature_child_of(usm_support, usm_all) + + netsnmp_feature_require(usm_support) + ++struct usmStateReference { ++ char *usr_name; ++ size_t usr_name_length; ++ u_char *usr_engine_id; ++ size_t usr_engine_id_length; ++ oid *usr_auth_protocol; ++ size_t usr_auth_protocol_length; ++ u_char *usr_auth_key; ++ size_t usr_auth_key_length; ++ oid *usr_priv_protocol; ++ size_t usr_priv_protocol_length; ++ u_char *usr_priv_key; ++ size_t usr_priv_key_length; ++ u_int usr_sec_level; ++}; ++ + oid usmNoAuthProtocol[10] = { NETSNMP_USMAUTH_BASE_OID, + NETSNMP_USMAUTH_NOAUTH }; + #ifndef NETSNMP_DISABLE_MD5