Patrick Williams | b48b7b4 | 2016-08-17 15:04:38 -0500 | [diff] [blame] | 1 | From 6b93e686bdb6a908d00595608646a05527a5326b Mon Sep 17 00:00:00 2001 |
| 2 | From: Li xin <lixin.fnst@cn.fujitsu.com> |
| 3 | Date: Fri, 21 Aug 2015 12:39:12 +0900 |
| 4 | Subject: [PATCH] the snmp_pdu_parse() function could leave incompletely parsed |
| 5 | varBind variables in the list of variables in case the parsing of the SNMP |
| 6 | PDU failed. If later processing tries to operate on the stale and |
| 7 | incompletely processed varBind (e.g. when printing the variables), this can |
| 8 | lead to e.g. crashes or, possibly, execution of arbitrary code |
| 9 | |
| 10 | Upstream-Status: Backport [net-snmp] |
| 11 | |
| 12 | Written-by: Robert Story |
| 13 | --- |
| 14 | snmplib/snmp_api.c | 53 ++++++++++++++++++++++++++++------------------------- |
| 15 | 1 file changed, 28 insertions(+), 25 deletions(-) |
| 16 | |
| 17 | diff --git a/snmplib/snmp_api.c b/snmplib/snmp_api.c |
| 18 | index 191debf..15a2d39 100644 |
| 19 | --- a/snmplib/snmp_api.c |
| 20 | +++ b/snmplib/snmp_api.c |
| 21 | @@ -4350,10 +4350,9 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length) |
| 22 | u_char type; |
| 23 | u_char msg_type; |
| 24 | u_char *var_val; |
| 25 | - int badtype = 0; |
| 26 | size_t len; |
| 27 | size_t four; |
| 28 | - netsnmp_variable_list *vp = NULL; |
| 29 | + netsnmp_variable_list *vp = NULL, *vplast = NULL; |
| 30 | oid objid[MAX_OID_LEN]; |
| 31 | u_char *p; |
| 32 | |
| 33 | @@ -4493,31 +4492,17 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length) |
| 34 | (ASN_SEQUENCE | ASN_CONSTRUCTOR), |
| 35 | "varbinds"); |
| 36 | if (data == NULL) |
| 37 | - return -1; |
| 38 | + goto fail; |
| 39 | |
| 40 | /* |
| 41 | * get each varBind sequence |
| 42 | */ |
| 43 | while ((int) *length > 0) { |
| 44 | - netsnmp_variable_list *vptemp; |
| 45 | - vptemp = (netsnmp_variable_list *) malloc(sizeof(*vptemp)); |
| 46 | - if (NULL == vptemp) { |
| 47 | - return -1; |
| 48 | - } |
| 49 | - if (NULL == vp) { |
| 50 | - pdu->variables = vptemp; |
| 51 | - } else { |
| 52 | - vp->next_variable = vptemp; |
| 53 | - } |
| 54 | - vp = vptemp; |
| 55 | + vp = SNMP_MALLOC_TYPEDEF(netsnmp_variable_list); |
| 56 | + if (NULL == vp) |
| 57 | + goto fail; |
| 58 | |
| 59 | - vp->next_variable = NULL; |
| 60 | - vp->val.string = NULL; |
| 61 | vp->name_length = MAX_OID_LEN; |
| 62 | - vp->name = NULL; |
| 63 | - vp->index = 0; |
| 64 | - vp->data = NULL; |
| 65 | - vp->dataFreeHook = NULL; |
| 66 | DEBUGDUMPSECTION("recv", "VarBind"); |
| 67 | data = snmp_parse_var_op(data, objid, &vp->name_length, &vp->type, |
| 68 | &vp->val_len, &var_val, length); |
| 69 | @@ -4604,7 +4589,7 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length) |
| 70 | vp->val.string = (u_char *) malloc(vp->val_len); |
| 71 | } |
| 72 | if (vp->val.string == NULL) { |
| 73 | - return -1; |
| 74 | + goto fail; |
| 75 | } |
| 76 | p = asn_parse_string(var_val, &len, &vp->type, vp->val.string, |
| 77 | &vp->val_len); |
| 78 | @@ -4619,7 +4604,7 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length) |
| 79 | vp->val_len *= sizeof(oid); |
| 80 | vp->val.objid = (oid *) malloc(vp->val_len); |
| 81 | if (vp->val.objid == NULL) { |
| 82 | - return -1; |
| 83 | + goto fail; |
| 84 | } |
| 85 | memmove(vp->val.objid, objid, vp->val_len); |
| 86 | break; |
| 87 | @@ -4631,7 +4616,7 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length) |
| 88 | case ASN_BIT_STR: |
| 89 | vp->val.bitstring = (u_char *) malloc(vp->val_len); |
| 90 | if (vp->val.bitstring == NULL) { |
| 91 | - return -1; |
| 92 | + goto fail; |
| 93 | } |
| 94 | p = asn_parse_bitstring(var_val, &len, &vp->type, |
| 95 | vp->val.bitstring, &vp->val_len); |
| 96 | @@ -4640,12 +4625,30 @@ snmp_pdu_parse(netsnmp_pdu *pdu, u_char * data, size_t * length) |
| 97 | break; |
| 98 | default: |
| 99 | snmp_log(LOG_ERR, "bad type returned (%x)\n", vp->type); |
| 100 | - badtype = -1; |
| 101 | + goto fail; |
| 102 | break; |
| 103 | } |
| 104 | DEBUGINDENTADD(-4); |
| 105 | + |
| 106 | + if (NULL == vplast) { |
| 107 | + pdu->variables = vp; |
| 108 | + } else { |
| 109 | + vplast->next_variable = vp; |
| 110 | + } |
| 111 | + vplast = vp; |
| 112 | + vp = NULL; |
| 113 | + |
| 114 | } |
| 115 | - return badtype; |
| 116 | + return 0; |
| 117 | + |
| 118 | + fail: |
| 119 | + DEBUGMSGTL(("recv", "error while parsing VarBindList\n")); |
| 120 | + /** if we were parsing a var, remove it from the pdu and free it */ |
| 121 | + if (vp) |
| 122 | + snmp_free_var(vp); |
| 123 | + |
| 124 | + return -1; |
| 125 | + |
| 126 | } |
| 127 | |
| 128 | /* |
| 129 | -- |
| 130 | 1.8.4.2 |
| 131 | |