Memory Leak in snmplib/scapi.c

I found a memory leak in the sc_hash()/sc_hash_type() function within snmplib/scapi.c.  The leak was identified by running Valgrind on a SNMP client application which queries v3 servers using SHA/AES.

Here's an example of the Valgrind memory leak trace (using net-snmp-5.7.2-28 from RHEL7):

==1069== 3,456 bytes in 72 blocks are definitely lost in loss record 1,568 of 1,616
==1069==    at 0x4C29BE3: malloc (vg_replace_malloc.c:299)
==1069==    by 0x70A63B7: CRYPTO_malloc (in /usr/lib64/libcrypto.so.1.0.2k)
==1069==    by 0x7161B06: EVP_MD_CTX_create (in /usr/lib64/libcrypto.so.1.0.2k)
==1069==    by 0x4EA3017: sc_hash (in /usr/lib64/libnetsnmp.so.31.0.2)
==1069==    by 0x4EA1CD8: hash_engineID (in /usr/lib64/libnetsnmp.so.31.0.2)
==1069==    by 0x4EA1DEC: search_enginetime_list (in /usr/lib64/libnetsnmp.so.31.0.2)
==1069==    by 0x4EA2256: set_enginetime (in /usr/lib64/libnetsnmp.so.31.0.2)
==1069==    by 0x4EC495E: usm_process_in_msg (in /usr/lib64/libnetsnmp.so.31.0.2)
==1069==    by 0x4EC58CA: usm_secmod_process_in_msg (in /usr/lib64/libnetsnmp.so.31.0.2)
==1069==    by 0x4E7B91D: snmpv3_parse (in /usr/lib64/libnetsnmp.so.31.0.2)
==1069==    by 0x4E7C1F6: ??? (in /usr/lib64/libnetsnmp.so.31.0.2)
==1069==    by 0x4E7CE94: ??? (in /usr/lib64/libnetsnmp.so.31.0.2)


My proposed patch against 5.8 to fix the leak (the fix for 5.7.x is very similar):

diff -ur net-snmp-5.8/snmplib/scapi.c net-snmp-5.8-new/snmplib/scapi.c
--- net-snmp-5.8/snmplib/scapi.c        2018-07-16 09:33:40.000000000 -0500
+++ net-snmp-5.8-new/snmplib/scapi.c    2018-10-09 17:32:16.123982354 -0500
@@ -967,7 +967,8 @@
 #endif
     if (!EVP_DigestInit(cptr, hashfn)) {
         /* requested hash function is not available */
-        return SNMPERR_SC_NOT_CONFIGURED;
+        rval = SNMPERR_SC_NOT_CONFIGURED;
+        goto sc_hash_type_quit;
     }

 /** pass the data */
@@ -976,16 +977,20 @@
 /** do the final pass */
     EVP_DigestFinal(cptr, MAC, &tmp_len);
     *MAC_len = tmp_len;
+
+sc_hash_type_quit:
+    if (cptr) {
 #if defined(HAVE_EVP_MD_CTX_FREE)
-    EVP_MD_CTX_free(cptr);
+        EVP_MD_CTX_free(cptr);
 #elif defined(HAVE_EVP_MD_CTX_DESTROY)
-    EVP_MD_CTX_destroy(cptr);
+        EVP_MD_CTX_destroy(cptr);
 #else
 #if !defined(OLD_DES)
-    EVP_MD_CTX_cleanup(cptr);
+        EVP_MD_CTX_cleanup(cptr);
 #endif
-    free(cptr);
+        free(cptr);
 #endif
+    }
     return (rval);

 #elif NETSNMP_USE_INTERNAL_CRYPTO



After applying the patch, Valgrind shows no memory leaks.

Comments or questions are welcome.  My company is planning to supply this patch to Redhat, but it would be nice to see it fixed upstream as well.


-Drew