diff -urNp x/agent/snmp_agent.c y/agent/snmp_agent.c
--- x/agent/snmp_agent.c 2019-09-12 10:07:52.536297633 +0200
+++ y/agent/snmp_agent.c 2019-09-12 10:10:10.900666975 +0200
@@ -1428,6 +1428,13 @@ free_agent_snmp_session(netsnmp_agent_se
DEBUGMSGTL(("verbose:asp", "asp %p reqinfo %p freed\n",
asp, asp->reqinfo));
+
+ /* Clean up securityStateRef here to prevent a double free */
+ if (asp->orig_pdu && asp->orig_pdu->securityStateRef)
+ snmp_free_securityStateRef(asp->orig_pdu);
+ if (asp->pdu && asp->pdu->securityStateRef)
+ snmp_free_securityStateRef(asp->pdu);
+
if (asp->orig_pdu)
snmp_free_pdu(asp->orig_pdu);
if (asp->pdu)
diff -urNp x/include/net-snmp/pdu_api.h y/include/net-snmp/pdu_api.h
--- x/include/net-snmp/pdu_api.h 2019-09-12 10:07:52.548297751 +0200
+++ y/include/net-snmp/pdu_api.h 2019-09-12 10:11:26.562411701 +0200
@@ -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 -urNp x/snmplib/snmp_api.c y/snmplib/snmp_api.c
--- x/snmplib/snmp_api.c 2019-09-12 10:07:52.570297968 +0200
+++ y/snmplib/snmp_api.c 2019-09-13 08:53:53.734689426 +0200
@@ -3837,6 +3837,42 @@ snmpv3_parse(netsnmp_pdu *pdu,
return SNMPERR_SUCCESS;
} /* end snmpv3_parse() */
+static void
+free_securityStateRef(netsnmp_pdu* pdu)
+{
+ 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);
+ } else {
+ snmp_log(LOG_ERR,
+ "Security Model %d can't free state references\n",
+ pdu->securityModel);
+ }
+ } else {
+ snmp_log(LOG_ERR,
+ "Can't find security model to free ptr: %d\n",
+ pdu->securityModel);
+ }
+ 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
@@ -3858,7 +3894,6 @@ snmpv3_make_report(netsnmp_pdu *pdu, int
oid *err_var;
int err_var_len;
int stat_ind;
- struct snmp_secmod_def *sptr;
switch (error) {
case SNMPERR_USM_UNKNOWNENGINEID:
@@ -3919,21 +3954,7 @@ snmpv3_make_report(netsnmp_pdu *pdu, int
* which cached values to use
*/
if (pdu->securityStateRef) {
- sptr = find_sec_mod(pdu->securityModel);
- if (sptr) {
- if (sptr->pdu_free_state_ref) {
- (*sptr->pdu_free_state_ref) (pdu->securityStateRef);
- } else {
- snmp_log(LOG_ERR,
- "Security Model %d can't free state references\n",
- pdu->securityModel);
- }
- } else {
- snmp_log(LOG_ERR,
- "Can't find security model to free ptr: %d\n",
- pdu->securityModel);
- }
- pdu->securityStateRef = NULL;
+ free_securityStateRef(pdu);
}
if (error == SNMPERR_USM_NOTINTIMEWINDOW) {
@@ -5192,7 +5213,6 @@ _sess_process_packet(void *sessp, netsnm
struct session_list *slp = (struct session_list *) sessp;
netsnmp_pdu *pdu;
netsnmp_request_list *rp, *orp = NULL;
- struct snmp_secmod_def *sptr;
int ret = 0, handled = 0;
DEBUGMSGTL(("sess_process_packet",
@@ -5262,21 +5282,7 @@ _sess_process_packet(void *sessp, netsnm
* Call the security model to free any securityStateRef supplied w/ msg.
*/
if (pdu->securityStateRef != NULL) {
- sptr = find_sec_mod(pdu->securityModel);
- if (sptr != NULL) {
- if (sptr->pdu_free_state_ref != NULL) {
- (*sptr->pdu_free_state_ref) (pdu->securityStateRef);
- } else {
- snmp_log(LOG_ERR,
- "Security Model %d can't free state references\n",
- pdu->securityModel);
- }
- } else {
- snmp_log(LOG_ERR,
- "Can't find security model to free ptr: %d\n",
- pdu->securityModel);
- }
- pdu->securityStateRef = NULL;
+ free_securityStateRef(pdu);
}
snmp_free_pdu(pdu);
return -1;
@@ -5287,21 +5293,7 @@ _sess_process_packet(void *sessp, netsnm
* Call USM to free any securityStateRef supplied with the message.
*/
if (pdu->securityStateRef) {
- sptr = find_sec_mod(pdu->securityModel);
- if (sptr) {
- if (sptr->pdu_free_state_ref) {
- (*sptr->pdu_free_state_ref) (pdu->securityStateRef);
- } else {
- snmp_log(LOG_ERR,
- "Security Model %d can't free state references\n",
- pdu->securityModel);
- }
- } else {
- snmp_log(LOG_ERR,
- "Can't find security model to free ptr: %d\n",
- pdu->securityModel);
- }
- pdu->securityStateRef = NULL;
+ free_securityStateRef(pdu);
}
for (rp = isp->requests; rp; orp = rp, rp = rp->next_request) {
@@ -5454,21 +5446,7 @@ _sess_process_packet(void *sessp, netsnm
*/
if (pdu != NULL && pdu->securityStateRef &&
pdu->command == SNMP_MSG_TRAP2) {
- sptr = find_sec_mod(pdu->securityModel);
- if (sptr) {
- if (sptr->pdu_free_state_ref) {
- (*sptr->pdu_free_state_ref) (pdu->securityStateRef);
- } else {
- snmp_log(LOG_ERR,
- "Security Model %d can't free state references\n",
- pdu->securityModel);
- }
- } else {
- snmp_log(LOG_ERR,
- "Can't find security model to free ptr: %d\n",
- pdu->securityModel);
- }
- pdu->securityStateRef = NULL;
+ free_securityStateRef(pdu);
}
if (!handled) {
diff -urNp x/snmplib/snmpusm.c y/snmplib/snmpusm.c
--- x/snmplib/snmpusm.c 2019-09-12 10:07:52.567297938 +0200
+++ y/snmplib/snmpusm.c 2019-09-12 10:57:52.780861077 +0200
@@ -206,16 +206,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_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) {
+ 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);
}
@@ -946,7 +950,6 @@ usm_generate_out_msg(int msgProcModel,
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;
}
@@ -998,7 +1001,6 @@ usm_generate_out_msg(int msgProcModel,
thePrivProtocolLength) == 1) {
DEBUGMSGTL(("usm", "Unsupported Security Level (%d)\n",
theSecLevel));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_UNSUPPORTEDSECURITYLEVEL;
}
@@ -1028,7 +1030,6 @@ usm_generate_out_msg(int msgProcModel,
&msgAuthParmLen, &msgPrivParmLen, &otstlen,
&seq_len, &msgSecParmLen) == -1) {
DEBUGMSGTL(("usm", "Failed calculating offsets.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_GENERICERROR;
}
@@ -1050,7 +1051,6 @@ usm_generate_out_msg(int msgProcModel,
ptr = *wholeMsg = globalData;
if (theTotalLength > *wholeMsgLen) {
DEBUGMSGTL(("usm", "Message won't fit in buffer.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_GENERICERROR;
}
@@ -1078,7 +1078,6 @@ usm_generate_out_msg(int msgProcModel,
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;
}
}
@@ -1091,7 +1090,6 @@ usm_generate_out_msg(int msgProcModel,
&ptr[privParamsOffset])
== -1)) {
DEBUGMSGTL(("usm", "Can't set DES-CBC salt.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_GENERICERROR;
}
}
@@ -1104,7 +1102,6 @@ usm_generate_out_msg(int msgProcModel,
&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
@@ -1132,7 +1129,6 @@ usm_generate_out_msg(int msgProcModel,
if ((encrypted_length != (theTotalLength - dataOffset))
|| (salt_length != msgPrivParmLen)) {
DEBUGMSGTL(("usm", "encryption length error.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_ENCRYPTIONERROR;
}
@@ -1268,7 +1264,6 @@ usm_generate_out_msg(int msgProcModel,
if (temp_sig == NULL) {
DEBUGMSGTL(("usm", "Out of memory.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_GENERICERROR;
}
@@ -1282,7 +1277,6 @@ usm_generate_out_msg(int msgProcModel,
SNMP_ZERO(temp_sig, temp_sig_len);
SNMP_FREE(temp_sig);
DEBUGMSGTL(("usm", "Signing failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_AUTHENTICATIONFAILURE;
}
@@ -1290,7 +1284,6 @@ usm_generate_out_msg(int msgProcModel,
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;
}
@@ -1304,7 +1297,6 @@ usm_generate_out_msg(int msgProcModel,
/*
* endif -- create keyed hash
*/
- usm_free_usmStateReference(secStateRef);
DEBUGMSGTL(("usm", "USM processing completed.\n"));
@@ -1458,7 +1450,6 @@ usm_rgenerate_out_msg(int msgProcModel,
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;
}
@@ -1511,7 +1502,6 @@ usm_rgenerate_out_msg(int msgProcModel,
DEBUGMSGTL(("usm", "Unsupported Security Level or type (%d)\n",
theSecLevel));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_UNSUPPORTEDSECURITYLEVEL;
}
@@ -1544,7 +1534,6 @@ usm_rgenerate_out_msg(int msgProcModel,
DEBUGMSGTL(("usm",
"couldn't malloc %d bytes for encrypted PDU\n",
(int)ciphertextlen));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_MALLOC;
}
@@ -1560,7 +1549,6 @@ usm_rgenerate_out_msg(int msgProcModel,
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;
}
@@ -1575,7 +1563,6 @@ usm_rgenerate_out_msg(int msgProcModel,
thePrivKeyLength - 8,
iv) == -1)) {
DEBUGMSGTL(("usm", "Can't set DES-CBC salt.\n"));
- usm_free_usmStateReference(secStateRef);
SNMP_FREE(ciphertext);
return SNMPERR_USM_GENERICERROR;
}
@@ -1594,7 +1581,6 @@ usm_rgenerate_out_msg(int msgProcModel,
scopedPdu, scopedPduLen,
ciphertext, &ciphertextlen) != SNMP_ERR_NOERROR) {
DEBUGMSGTL(("usm", "encryption error.\n"));
- usm_free_usmStateReference(secStateRef);
SNMP_FREE(ciphertext);
return SNMPERR_USM_ENCRYPTIONERROR;
}
@@ -1614,7 +1600,6 @@ usm_rgenerate_out_msg(int msgProcModel,
ciphertext, ciphertextlen);
if (rc == 0) {
DEBUGMSGTL(("usm", "Encryption failed.\n"));
- usm_free_usmStateReference(secStateRef);
SNMP_FREE(ciphertext);
return SNMPERR_USM_ENCRYPTIONERROR;
}
@@ -1654,7 +1639,6 @@ usm_rgenerate_out_msg(int msgProcModel,
DEBUGINDENTLESS();
if (rc == 0) {
DEBUGMSGTL(("usm", "building privParams failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1675,7 +1659,6 @@ usm_rgenerate_out_msg(int msgProcModel,
DEBUGINDENTLESS();
if (rc == 0) {
DEBUGMSGTL(("usm", "building authParams failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1698,7 +1681,6 @@ usm_rgenerate_out_msg(int msgProcModel,
DEBUGINDENTLESS();
if (rc == 0) {
DEBUGMSGTL(("usm", "building authParams failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1714,7 +1696,6 @@ usm_rgenerate_out_msg(int msgProcModel,
if (rc == 0) {
DEBUGMSGTL(("usm",
"building msgAuthoritativeEngineTime failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1730,7 +1711,6 @@ usm_rgenerate_out_msg(int msgProcModel,
if (rc == 0) {
DEBUGMSGTL(("usm",
"building msgAuthoritativeEngineBoots failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1742,7 +1722,6 @@ usm_rgenerate_out_msg(int msgProcModel,
DEBUGINDENTLESS();
if (rc == 0) {
DEBUGMSGTL(("usm", "building msgAuthoritativeEngineID failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1755,7 +1734,6 @@ usm_rgenerate_out_msg(int msgProcModel,
*offset - sp_offset);
if (rc == 0) {
DEBUGMSGTL(("usm", "building usm security parameters failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1769,7 +1747,6 @@ usm_rgenerate_out_msg(int msgProcModel,
if (rc == 0) {
DEBUGMSGTL(("usm", "building msgSecurityParameters failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1779,7 +1756,6 @@ usm_rgenerate_out_msg(int msgProcModel,
while ((*wholeMsgLen - *offset) < globalDataLen) {
if (!asn_realloc(wholeMsg, wholeMsgLen)) {
DEBUGMSGTL(("usm", "building global data failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
}
@@ -1795,7 +1771,6 @@ usm_rgenerate_out_msg(int msgProcModel,
ASN_CONSTRUCTOR), *offset);
if (rc == 0) {
DEBUGMSGTL(("usm", "building master packet sequence failed.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_TOO_LONG;
}
@@ -1813,7 +1788,6 @@ usm_rgenerate_out_msg(int msgProcModel,
if (temp_sig == NULL) {
DEBUGMSGTL(("usm", "Out of memory.\n"));
- usm_free_usmStateReference(secStateRef);
return SNMPERR_USM_GENERICERROR;
}
@@ -1824,14 +1798,12 @@ usm_rgenerate_out_msg(int msgProcModel,
!= 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;
}
@@ -1842,7 +1814,6 @@ usm_rgenerate_out_msg(int msgProcModel,
/*
* endif -- create keyed hash
*/
- usm_free_usmStateReference(secStateRef);
DEBUGMSGTL(("usm", "USM processing completed.\n"));
return SNMPERR_SUCCESS;
} /* end usm_rgenerate_out_msg() */