From 6b93e686bdb6a908d00595608646a05527a5326b Mon Sep 17 00:00:00 2001
From: Li xin <lixin.fnst@cn.fujitsu.com>
Date: Fri, 21 Aug 2015 12:39:12 +0900
Subject: [PATCH] the snmp_pdu_parse() function could leave incompletely parsed
 varBind variables in the list of variables in case the parsing of the SNMP
 PDU failed. If later processing tries to operate on the stale and
 incompletely processed varBind (e.g. when printing the variables), this can
 lead to e.g. crashes or, possibly, execution of arbitrary code

Upstream-Status: Backport [net-snmp]

Written-by: Robert Story
---
 snmplib/snmp_api.c | 53 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/snmplib/snmp_api.c b/snmplib/snmp_api.c
index 191debf..15a2d39 100644
--- a/snmplib/snmp_api.c
+++ b/snmplib/snmp_api.c
@@ -4350,10 +4350,9 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length)
     u_char          type;
     u_char          msg_type;
     u_char         *var_val;
-    int             badtype = 0;
     size_t          len;
     size_t          four;
-    netsnmp_variable_list *vp = NULL;
+    netsnmp_variable_list *vp = NULL, *vplast = NULL;
     oid             objid[MAX_OID_LEN];
     u_char         *p;
 
@@ -4493,31 +4492,17 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length)
                               (ASN_SEQUENCE | ASN_CONSTRUCTOR),
                               "varbinds");
     if (data == NULL)
-        return -1;
+        goto fail;
 
     /*
      * get each varBind sequence 
      */
     while ((int) *length > 0) {
-        netsnmp_variable_list *vptemp;
-        vptemp = (netsnmp_variable_list *) malloc(sizeof(*vptemp));
-        if (NULL == vptemp) {
-            return -1;
-        }
-        if (NULL == vp) {
-            pdu->variables = vptemp;
-        } else {
-            vp->next_variable = vptemp;
-        }
-        vp = vptemp;
+        vp = SNMP_MALLOC_TYPEDEF(netsnmp_variable_list);
+        if (NULL == vp)
+            goto fail;
 
-        vp->next_variable = NULL;
-        vp->val.string = NULL;
         vp->name_length = MAX_OID_LEN;
-        vp->name = NULL;
-        vp->index = 0;
-        vp->data = NULL;
-        vp->dataFreeHook = NULL;
         DEBUGDUMPSECTION("recv", "VarBind");
         data = snmp_parse_var_op(data, objid, &vp->name_length, &vp->type,
                                  &vp->val_len, &var_val, length);
@@ -4604,7 +4589,7 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length)
                 vp->val.string = (u_char *) malloc(vp->val_len);
             }
             if (vp->val.string == NULL) {
-                return -1;
+                goto fail;
             }
             p = asn_parse_string(var_val, &len, &vp->type, vp->val.string,
                              &vp->val_len);
@@ -4619,7 +4604,7 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length)
             vp->val_len *= sizeof(oid);
             vp->val.objid = (oid *) malloc(vp->val_len);
             if (vp->val.objid == NULL) {
-                return -1;
+                goto fail;
             }
             memmove(vp->val.objid, objid, vp->val_len);
             break;
@@ -4631,7 +4616,7 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length)
         case ASN_BIT_STR:
             vp->val.bitstring = (u_char *) malloc(vp->val_len);
             if (vp->val.bitstring == NULL) {
-                return -1;
+                goto fail;
             }
             p = asn_parse_bitstring(var_val, &len, &vp->type,
                                 vp->val.bitstring, &vp->val_len);
@@ -4640,12 +4625,30 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length)
             break;
         default:
             snmp_log(LOG_ERR, "bad type returned (%x)\n", vp->type);
-            badtype = -1;
+            goto fail;
             break;
         }
         DEBUGINDENTADD(-4);
+
+        if (NULL == vplast) {
+            pdu->variables = vp;
+        } else {
+            vplast->next_variable = vp;
+        }
+        vplast = vp;
+        vp = NULL;
+
     }
-    return badtype;
+    return 0;
+
+  fail:
+    DEBUGMSGTL(("recv", "error while parsing VarBindList\n"));
+    /** if we were parsing a var, remove it from the pdu and free it */
+    if (vp)
+        snmp_free_var(vp);
+
+    return -1;
+
 }
 
 /*
-- 
1.8.4.2

