#2887 memory leak in buffer allocated in agent_trap.c

[lijo]

A memory leak is observed in Solaris( net-snmp version 5.7.3) while running some testsuites involving creation of traps.
The libumem memory allocation stack is given below

libumem.so.1umem_cache_alloc_debug+0x155 libumem.so.1umem_cache_alloc+0x1fd
libumem.so.1umem_alloc+0x5a libumem.so.1malloc+0x4c
libnetsnmp.so.30.0.3netsnmp_parse_args+0x120f libnetsnmpagent.so.30.0.3snmpd_parse_config_trapsess+0x1d7
libnetsnmp.so.30.0.3run_config_handler+0x249 libnetsnmp.so.30.0.3read_config+0xb3e
libnetsnmp.so.30.0.3read_config_files_in_path+0x6e4 libnetsnmp.so.30.0.3read_config_files+0xea
libnetsnmp.so.30.0.3read_configs+0xe1 libak.so.1ak_snmp_trap+0x12d
libak.so.1akx_invoke+0xc7 libak.so.1akx_call+0x99
libak.so.1akx_rpc_svc+0x3b99 libak.so.1ak_rpc_svc_door_process+0x39
libak.so.1ak_rpc_svc+0x125 libak.so.1ak_sidedoor+0xc0
libak.so.1`ak_door_serve+0xc1

The leaked buffer is allocated by the following code in the
"netsnmp_parse_args" function in snmp_parse_args.c
466 case 'e':{
467 size_t ebuf_len = 32, eout_len = 0;
468 u_char ebuf = (u_char )malloc(ebuf_len);

which is being called by the "snmpd_parse_config_trapsess" function.

This is being assigned at snmplib/snmp_parse_args.c#485
485 session->securityEngineID = ebuf;

It looks like this session variable is just being used as a template to create a new session at
agent/agent_trap.c#1254

<---------------SNIP ------------------------------>
1239 netsnmp_parse_args(argn, argv, &session, "C:", trapOptProc,
1240 NETSNMP_PARSE_ARGS_NOLOGGING |
1241 NETSNMP_PARSE_ARGS_NOZERO);
1242
1243 transport = netsnmp_transport_open_client("snmptrap",
session.peername);
1244 if (transport == NULL) {
1245 config_perror("snmpd: failed to parse this line.");
1246 return;
1247 }
1248 if ((rc = netsnmp_sess_config_and_open_transport(&session,
transport))
1249 != SNMPERR_SUCCESS) {
1250 session.s_snmp_errno = rc;
1251 session.s_errno = 0;
1252 return;
1253 }
1254 ss = snmp_add(&session, transport, NULL, NULL);

Hence it seems safe to free the buffer allocated
earlier(session->securityEngineID) after the "snmp_add" call
i.e if(session.securityEngineIDLen > 0)
SNMP_FREE(session.securityEngineID);
could be a fix. This should also be added to the error checks at lines 1244
and 1248.

I've tried this fix on the 5.7.3 and it looks the issue is solved, I assume the issue is still present in the 5.8 version as the relevant code has not changed much.
Attached patches for both the versions.

5.7.3 patch

---

--- net-snmp-5.7.3.old/agent/agent_trap.c 2014-12-08 12:23:22.000000000 +0000
+++ net-snmp-5.7.3/agent/agent_trap.c 2018-09-17 02:31:09.691193284 +0000
@@ -1243,18 +1243,28 @@
transport = netsnmp_transport_open_client("snmptrap", session.peername);
if (transport == NULL) {
config_perror("snmpd: failed to parse this line.");

• for (; argn > 0; argn--)
• free(argv[argn - 1]);
• if(session.securityEngineIDLen > 0)
• SNMP_FREE(session.securityEngineID);
  return;
  }
  if ((rc = netsnmp_sess_config_and_open_transport(&session, transport))
  != SNMPERR_SUCCESS) {
  session.s_snmp_errno = rc;
  session.s_errno = 0;
• for (; argn > 0; argn--)
• free(argv[argn - 1]);
• if(session.securityEngineIDLen > 0)
• SNMP_FREE(session.securityEngineID);
  return;
  }
  ss = snmp_add(&session, transport, NULL, NULL);
  for (; argn > 0; argn--) {
  free(argv[argn - 1]);
  }
• if(session.securityEngineIDLen > 0)

• SNMP_FREE(session.securityEngineID);

  if (!ss) {
  config_perror

5.8 patch

---

--- net-snmp-5.8.old/agent/agent_trap.c 2018-09-19 15:47:24.329390437 +0530
+++ net-snmp-5.8.old/agent/agent_trap.c 2018-09-19 15:56:46.552246462 +0530
@@ -1708,6 +1708,8 @@
config_perror("snmpd: protocol version disabled at runtime");
for (; argn > 0; argn--)
free(argv[argn - 1]);

• if(session.securityEngineIDLen > 0)
• SNMP_FREE(session.securityEngineID);
  goto cleanup;
  }

@@ -1731,6 +1733,8 @@
config_perror("snmpd: failed to parse this line.");
for (; argn > 0; argn--)
free(argv[argn - 1]);

• if(session.securityEngineIDLen > 0)
• SNMP_FREE(session.securityEngineID);
  goto cleanup;
  }
  if ((rc = netsnmp_sess_config_and_open_transport(&session, transport))
  @@ -1739,11 +1743,15 @@
  session.s_errno = 0;
  for (; argn > 0; argn--)
  free(argv[argn - 1]);
• if(session.securityEngineIDLen > 0)
• SNMP_FREE(session.securityEngineID);
  goto cleanup;
  }
  ss = snmp_add(&session, transport, NULL, NULL);
  for (; argn > 0; argn--)
  free(argv[argn - 1]);
• if(session.securityEngineIDLen > 0)

• SNMP_FREE(session.securityEngineID);

  if (!ss) {
  config_perror

[lijo]

Attaching patch for the 5.7.3 version also.

[Bart Van Assche]

A fix has been applied on the v5.8 and master branches. Please retest.