Add a "tftp_strnlen()" routine that
authorguy <guy@f5534014-38df-0310-8fa8-9805f1628bb7>
Sun, 24 Dec 2000 20:33:04 +0000 (20:33 +0000)
committerguy <guy@f5534014-38df-0310-8fa8-9805f1628bb7>
Sun, 24 Dec 2000 20:33:04 +0000 (20:33 +0000)
1) checks to make sure that the terminating '\0' is found in the
   string, and throws a BoundsError exception if it isn't (TFTP
   packets should fit in a single frame, so if the '\0' isn't
   found, that's an error);

2) adds 1 to the length to include the trailing '\0';

and use it to find all string lengths, so that we properly handle short
or malformed frames.

git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@2778 f5534014-38df-0310-8fa8-9805f1628bb7

packet-tftp.c

index 5161344e6d51382eef3e02a93ffb582794db8c8d..2c51258b90b70fca6835a3d272006cfc26542f3b 100644 (file)
@@ -5,7 +5,7 @@
  * Craig Newell <CraigN@cheque.uq.edu.au>
  *     RFC2347 TFTP Option Extension
  *
- * $Id: packet-tftp.c,v 1.19 2000/12/02 08:41:08 guy Exp $
+ * $Id: packet-tftp.c,v 1.20 2000/12/24 20:33:04 guy Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@zing.org>
@@ -87,6 +87,7 @@ static const value_string tftp_error_code_vals[] = {
 };
 
 static void tftp_dissect_options(tvbuff_t *tvb, int offset, proto_tree *tree);
+static gint tftp_strnlen(tvbuff_t *tvb, gint offset);
 
 static void
 dissect_tftp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
@@ -151,28 +152,28 @@ dissect_tftp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
            
          switch (opcode) {
          case RRQ:
-           i1 = tvb_strnlen(tvb, offset, -1);
+           i1 = tftp_strnlen(tvb, offset);
            proto_tree_add_item(tftp_tree, hf_tftp_source_file,
-                           tvb, offset, i1 + 1, FALSE);
-           offset += i1 + 1;
+                           tvb, offset, i1, FALSE);
+           offset += i1;
 
-           i1 = tvb_strnlen(tvb, offset, -1);
+           i1 = tftp_strnlen(tvb, offset);
            ti = proto_tree_add_item(tftp_tree, hf_tftp_transfer_type,
-                           tvb, offset, i1 + 1, FALSE);
-           offset += i1 + 1;
+                           tvb, offset, i1, FALSE);
+           offset += i1;
 
            tftp_dissect_options(tvb, offset, tftp_tree);
            break;
          case WRQ:
-           i1 = tvb_strnlen(tvb, offset, -1);
+           i1 = tftp_strnlen(tvb, offset);
            proto_tree_add_item(tftp_tree, hf_tftp_destination_file,
-                           tvb, offset, i1 + 1, FALSE);
-           offset += i1 + 1;
+                           tvb, offset, i1, FALSE);
+           offset += i1;
 
-           i1 = tvb_strnlen(tvb, offset, -1);
+           i1 = tftp_strnlen(tvb, offset);
            ti = proto_tree_add_item(tftp_tree, hf_tftp_transfer_type,
-                           tvb, offset, i1 + 1, FALSE);
-           offset += i1 + 1;
+                           tvb, offset, i1, FALSE);
+           offset += i1;
 
            tftp_dissect_options(tvb, offset, tftp_tree);
            break;
@@ -193,9 +194,9 @@ dissect_tftp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                            FALSE);
            offset += 2;
 
-           i1 = tvb_strnlen(tvb, offset, -1);
+           i1 = tftp_strnlen(tvb, offset);
            proto_tree_add_item(tftp_tree, hf_tftp_error_string, tvb, offset,
-               i1 + 1, FALSE);
+               i1, FALSE);
            break;
          case OACK:
            tftp_dissect_options(tvb, offset, tftp_tree);
@@ -212,19 +213,48 @@ dissect_tftp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 static void
 tftp_dissect_options(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-       int i1, i2;
+       int option_len, value_len;
+       int value_offset;
 
        while (tvb_offset_exists(tvb, offset)) {
-         i1 = tvb_strnlen(tvb, offset, -1);    /* length of option */
-         i2 = tvb_strnlen(tvb, offset+i1+1, -1);       /* length of value */
-         proto_tree_add_text(tree, tvb, offset, i1+i2+2,
+         option_len = tftp_strnlen(tvb, offset);       /* length of option */
+         value_offset = offset + option_len;
+         value_len = tftp_strnlen(tvb, value_offset);  /* length of value */
+         proto_tree_add_text(tree, tvb, offset, option_len+value_len,
                  "Option: %.*s = %.*s",
-                 i1, tvb_get_ptr(tvb, offset, i1),
-                 i2, tvb_get_ptr(tvb, offset+i1+1, i2));
-         offset += i1 + i2 + 2;
+                 option_len - 1, tvb_get_ptr(tvb, offset, option_len - 1),
+                 value_len - 1, tvb_get_ptr(tvb, value_offset, value_len - 1));
+         offset += option_len + value_len;
        }
 }
 
+/*
+ * Find the length of a null-terminated string in a TFTP packet; if we
+ * don't find the '\0', throw an exception, as it means that either
+ * we didn't capture enough of the frame, or the frame is malformed.
+ *
+ * XXX - we'd like to know *which* exception to throw....
+ *
+ * XXX - this should probably be a standard tvbuff accessor.
+ *
+ * Add 1 to the resulting length, so that it includes the '\0'.
+ */
+static gint
+tftp_strnlen(tvbuff_t *tvb, gint offset)
+{
+       gint len;
+
+       len = tvb_strnlen(tvb, offset, -1);
+       if (len == -1) {
+           /*
+            * No '\0' found before the end of the tvbuff; throw an
+            * exception.
+            */
+           THROW(BoundsError);
+       }
+       return len + 1;
+}
+
 void
 proto_register_tftp(void)
 {