Fixed the infinite-loop problem in the DNS dissector, at least for
authorGilbert Ramirez <gram@alumni.rice.edu>
Thu, 7 Oct 1999 02:26:45 +0000 (02:26 -0000)
committerGilbert Ramirez <gram@alumni.rice.edu>
Thu, 7 Oct 1999 02:26:45 +0000 (02:26 -0000)
the random packets I generated. I'm not convinced that all the problems
are gone. We now:

1. Check that the bytes are indded in the frame before accessing them
in dissect_dns_query() and dissect_dns_answer(). If not, we
return 0, which means "0-byte increment".

2. Check the return value of the two functions above in
dissect_query_records() and dissect_answer_records(), which have
loops that call those two functions above. If a 0-byte
increment is found, the loop is broken to avoid an infinite loop.

svn path=/trunk/; revision=778

packet-dns.c

index 0cb0e9841b6a0fbf8f934f4ded9d56318560a4a7..d8d5efa6120b8ce1c06db7f7872a61395f8cd954 100644 (file)
@@ -1,7 +1,7 @@
 /* packet-dns.c
  * Routines for DNS packet disassembly
  *
- * $Id: packet-dns.c,v 1.21 1999/09/21 07:15:38 guy Exp $
+ * $Id: packet-dns.c,v 1.22 1999/10/07 02:26:45 gram Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@zing.org>
@@ -455,6 +455,10 @@ dissect_dns_query(const u_char *dns_data_ptr, const u_char *pd, int offset,
     &type, &class);
   dptr += len;
 
+  if (! BYTES_ARE_IN_FRAME(offset, len) ) {
+         return 0;
+  }
+
   type_name = dns_type_name(type);
   class_name = dns_class_name(class);
   long_type_name = dns_long_type_name(type);
@@ -523,6 +527,10 @@ dissect_dns_answer(const u_char *dns_data_ptr, const u_char *pd, int offset,
     &type, &class);
   dptr += len;
 
+  if (! BYTES_ARE_IN_FRAME(offset, len) ) {
+         return 0;
+  }
+
   type_name = dns_type_name(type);
   class_name = dns_class_name(class);
   long_type_name = dns_long_type_name(type);
@@ -735,15 +743,20 @@ static int
 dissect_query_records(const u_char *dns_data_ptr, int count, const u_char *pd, 
                      int cur_off, proto_tree *dns_tree)
 {
-  int start_off;
+  int start_off, add_off;
   proto_tree *qatree;
   proto_item *ti;
   
   start_off = cur_off;
   ti = proto_tree_add_text(dns_tree, start_off, 0, "Queries");
   qatree = proto_item_add_subtree(ti, ETT_DNS_QRY);
-  while (count-- > 0)
-    cur_off += dissect_dns_query(dns_data_ptr, pd, cur_off, qatree);
+  while (count-- > 0) {
+    add_off = dissect_dns_query(dns_data_ptr, pd, cur_off, qatree);
+    if (add_off <= 0) {
+           break;
+    }
+    cur_off += add_off;
+  }
   proto_item_set_len(ti, cur_off - start_off);
 
   return cur_off - start_off;
@@ -756,15 +769,20 @@ dissect_answer_records(const u_char *dns_data_ptr, int count,
                        const u_char *pd, int cur_off, proto_tree *dns_tree,
                        char *name)
 {
-  int start_off;
+  int start_off, add_off;
   proto_tree *qatree;
   proto_item *ti;
   
   start_off = cur_off;
   ti = proto_tree_add_text(dns_tree, start_off, 0, name);
   qatree = proto_item_add_subtree(ti, ETT_DNS_ANS);
-  while (count-- > 0)
-    cur_off += dissect_dns_answer(dns_data_ptr, pd, cur_off, qatree);
+  while (count-- > 0) {
+    add_off = dissect_dns_answer(dns_data_ptr, pd, cur_off, qatree);
+    if (add_off <= 0) {
+           break;
+    }
+    cur_off += add_off;
+  }
   proto_item_set_len(ti, cur_off - start_off);
 
   return cur_off - start_off;