Ethereal-dev: [Ethereal-dev] Crash in SNMP capture (security bug?)

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Yaniv Kaul <ykaul@xxxxxxxxxxxx>
Date: Mon, 28 Jun 2004 19:35:28 +0200
Attached please find a packet capture that causes crash in SNMP code.

The faulty code is in packet-snmp.c:
       ret = asn1_octet_string_decode (&asn1, &community,
           &community_length, &length);
       if (tree) {
           dissect_snmp2u_parameters(snmp_tree, tvb, offset, length,
               community, community_length);
       }

There's no check on 'ret' value, and so there's a call to 'dissect_snmp2u_parameters()' with community as NULL (since the asn1_octect_string_decode() fails in this malformed packet).

Attached please find a diff that fixes this problem.
I went over (briefly) on all packet-snmp.c, and it seems this is the ONLY place we don't check the 'ret' value.

Attachment: snmp.fix.cap
Description: Binary data

--- packet-snmp.orig.c	2004-06-28 19:22:56.000000000 +0200
+++ packet-snmp.c	2004-06-28 19:27:54.000000000 +0200
@@ -1703,6 +1703,11 @@
 	case SNMP_VERSION_2u:
 		ret = asn1_octet_string_decode (&asn1, &community,
 		    &community_length, &length);
+		if (ret != ASN1_ERR_NOERROR) {
+			dissect_snmp_parse_error(tvb, offset, pinfo, snmp_tree,
+				"community (2u)", ret);
+			return message_length;
+		}
 		if (tree) {
 			dissect_snmp2u_parameters(snmp_tree, tvb, offset, length,
 			    community, community_length);