Attached is a patch (against the repo) that fixes a fatal bug in
packet-ldap.c and a frame that triggers the failure.
According to RFC 1777, an LDAP message is composed of
SEQUENCE {
messageID MessageID,
protocolOp CHOICE {
...
By chance, an LDAP frame may start with a SEQUENCE, but not one that
represents the start of an LDAP message. Since packet-ldap.c didn't
check the return value of read_integer, it didn't notice that
read_integer failed when a MessageID wasn't present (which meant that
it wasn't a valid LDAP message). This eventually led to a segfault
when strlen(3) was called on a NULL pointer.
A useful exercise would be to add similar validation logic to the rest
of the dissector as well; have to save that for another patch ;-)
cheers,
--Scott
--
Scott Renfro <scott@xxxxxxxxxx> +1 650 906 9618Index: packet-ldap.c
===================================================================
RCS file: /cvsroot/ethereal/packet-ldap.c,v
retrieving revision 1.24
diff -u -u -r1.24 packet-ldap.c
--- packet-ldap.c 2001/04/15 07:35:26 1.24
+++ packet-ldap.c 2001/05/08 07:47:59
@@ -804,6 +804,9 @@
read_string(a, 0, -1, 0, &string1, ASN1_UNI, ASN1_OTS);
read_string(a, 0, -1, 0, &string2, ASN1_UNI, ASN1_OTS);
+ if (string1 == 0 && string2 == 0) /* read_string failed */
+ return 1;
+
length = 2 + strlen(string1) + strlen(string2);
compare = g_malloc0(length);
snprintf(compare, length, "%s=%s", string1, string2);
@@ -906,7 +909,13 @@
}
message_id_start = a.offset;
- read_integer(&a, 0, -1, 0, &messageId, ASN1_INT);
+ if (read_integer(&a, 0, -1, 0, &messageId, ASN1_INT))
+ {
+ if (ldap_tree)
+ proto_tree_add_text(ldap_tree, tvb, message_id_start, 1,
+ "Invalid LDAP packet (No Message ID)");
+ break;
+ }
message_id_length = a.offset - message_id_start;
start = a.offset;
Attachment:
ethereal-ldap-no-msg-id.pcap
Description: Binary data
