From 8e642c32bb59e4b93f514174bb8203d9aee93d2e Mon Sep 17 00:00:00 2001 From: Jian Liu Date: Tue, 7 Jul 2015 09:17:30 +0800 Subject: net-snmp: don't return incompletely parsed varbinds 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. The snmp_pdu_parse() function stores varBind variables in a list of netsnmp_variable_list structures. Each time the function parses a new varBind, a new netsnmp_variable_list item is allocated on the heap and linked to the list of variables. The problem is that this item is not removed from the list, even if snmp_pdu_parse() fails to complete the parsing. The "type" member of the stale netsnmp_variable_list is not properly initialized in case snmp_pdu_parse() returns early from the parsing. However, the "type" member is used to determine later code paths, which is why we see crashes in a variety of functions, although the root cause for all of these is the same. This patch come from http://sourceforge.net/p/net-snmp/code/ci/f23bcd3ac6ddee5d0a48f9703007ccc738914791/ Written-by: Robert Story Signed-off-by: Jian Liu Signed-off-by: Martin Jansa Signed-off-by: Joe MacDonald --- .../dont-return-incompletely-parsed-varbinds.patch | 128 +++++++++++++++++++++ .../recipes-protocols/net-snmp/net-snmp_5.7.2.1.bb | 1 + 2 files changed, 129 insertions(+) create mode 100644 meta-networking/recipes-protocols/net-snmp/net-snmp/dont-return-incompletely-parsed-varbinds.patch (limited to 'meta-networking') diff --git a/meta-networking/recipes-protocols/net-snmp/net-snmp/dont-return-incompletely-parsed-varbinds.patch b/meta-networking/recipes-protocols/net-snmp/net-snmp/dont-return-incompletely-parsed-varbinds.patch new file mode 100644 index 0000000000..04f2110f39 --- /dev/null +++ b/meta-networking/recipes-protocols/net-snmp/net-snmp/dont-return-incompletely-parsed-varbinds.patch @@ -0,0 +1,128 @@ +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 + +diff -Nur net-snmp-5.7.2.1.orig/snmplib/snmp_api.c net-snmp-5.7.2.1/snmplib/snmp_api.c +--- net-snmp-5.7.2.1.orig/snmplib/snmp_api.c 2015-05-27 11:25:11.563747471 +0800 ++++ net-snmp-5.7.2.1/snmplib/snmp_api.c 2015-05-27 13:27:27.724748201 +0800 +@@ -4345,10 +4345,9 @@ + 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]; + + /* +@@ -4487,38 +4486,24 @@ + (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); + if (data == NULL) +- return -1; ++ goto fail; + if (snmp_set_var_objid(vp, objid, vp->name_length)) +- return -1; ++ goto fail; + + len = MAX_PACKET_LENGTH; + DEBUGDUMPHEADER("recv", "Value"); +@@ -4583,7 +4568,7 @@ + vp->val.string = (u_char *) malloc(vp->val_len); + } + if (vp->val.string == NULL) { +- return -1; ++ goto fail; + } + asn_parse_string(var_val, &len, &vp->type, vp->val.string, + &vp->val_len); +@@ -4594,7 +4579,7 @@ + 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; +@@ -4606,19 +4591,35 @@ + case ASN_BIT_STR: + vp->val.bitstring = (u_char *) malloc(vp->val_len); + if (vp->val.bitstring == NULL) { +- return -1; ++ goto fail; + } + asn_parse_bitstring(var_val, &len, &vp->type, + vp->val.bitstring, &vp->val_len); + 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; + } + + /* diff --git a/meta-networking/recipes-protocols/net-snmp/net-snmp_5.7.2.1.bb b/meta-networking/recipes-protocols/net-snmp/net-snmp_5.7.2.1.bb index 93d3a941ea..2d9d9740f1 100644 --- a/meta-networking/recipes-protocols/net-snmp/net-snmp_5.7.2.1.bb +++ b/meta-networking/recipes-protocols/net-snmp/net-snmp_5.7.2.1.bb @@ -21,6 +21,7 @@ SRC_URI = "${SOURCEFORGE_MIRROR}/net-snmp/net-snmp-${PV}.zip \ file://net-snmp-testing-add-the-output-format-for-ptest.patch \ file://run-ptest \ file://0001-Fix-CVE-2014-2285.patch \ + file://dont-return-incompletely-parsed-varbinds.patch \ " SRC_URI[md5sum] = "a2c83518648b0f2a5d378625e45c0e18" -- cgit v1.2.3-54-g00ecf