Add a "tvb_ensure_bytes_exist()", which is like "tvb_bytes_exist()" only

it throws the appropriate exception if the bytes don't exist.  Use it in
the GIOP and ASN.1 code to check whether the bytes to be copied to a
buffer exist before allocating the buffer.

Make "check_offset_length_no_exception()" check for an overflow, so that
it can be used in "tvb_ensure_bytes_exist()" and do all the checking
that the code "tvb_ensure_bytes_exist()" replaces did.

Make "get_CDR_wchar()" return a "gint", so that if the length octet it
fetched has a value between 128 and 255, the length can be returned
correctly.

Fix some comments not to specify the exception thrown by various
routines that can throw various exceptions.

svn path=/trunk/; revision=5453

5 files changed, 91 insertions, 51 deletions

diff --git a/asn1.c b/asn1.c
index a452de42e8..fbe7284f8f 100644
--- a/asn1.c
+++ b/asn1.c
@@ -1,7 +1,7 @@
 /* asn1.c
  * Routines for ASN.1 BER dissection
  *
- * $Id: asn1.c,v 1.12 2002/03/05 09:18:58 guy Exp $
+ * $Id: asn1.c,v 1.13 2002/05/13 01:24:45 guy Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@ethereal.com>
@@ -655,27 +655,15 @@ asn1_string_value_decode ( ASN1_SCK *asn1, int enc_len, guchar **octets)
     int          eoc;
     guchar       *ptr;
 
-    eoc = asn1->offset + enc_len;
-
-    /*
-     * Check for an overflow, and clamp "eoc" at the maximum if we
-     * get it.
-     */
-    if (eoc < asn1->offset || eoc < 0)
-	eoc = INT_MAX;
-
     /*
      * First, make sure the entire string is in the tvbuff, and throw
      * an exception if it isn't.  If the length is bogus, this should
      * keep us from trying to allocate an immensely large buffer.
      * (It won't help if the length is *valid* but immensely large,
      * but that's another matter.)
-     *
-     * We do that by attempting to fetch the last byte (if the length
-     * isn't 0).
      */
     if (enc_len != 0) {
-	tvb_get_guint8(asn1->tvb, eoc - 1);
+    	tvb_ensure_bytes_exist(asn1->tvb, asn1->offset, enc_len);
 	*octets = g_malloc (enc_len);
     } else {
 	/*
@@ -686,6 +674,8 @@ asn1_string_value_decode ( ASN1_SCK *asn1, int enc_len, guchar **octets)
 	 */
 	*octets = g_malloc (1);
     }
+
+    eoc = asn1->offset + enc_len;
     ptr = *octets;
     while (asn1->offset < eoc) {
 	ret = asn1_octet_decode (asn1, (guchar *)ptr++);
@@ -832,27 +822,17 @@ asn1_oid_value_decode ( ASN1_SCK *asn1, int enc_len, subid_t **oid, guint *len)
     guint        size;
     subid_t      *optr;
 
-    eoc = asn1->offset + enc_len;
-
-    /*
-     * Check for an overflow, and clamp "eoc" at the maximum if we
-     * get it.
-     */
-    if (eoc < asn1->offset || eoc < 0)
-	eoc = INT_MAX;
-
     /*
      * First, make sure the entire string is in the tvbuff, and throw
      * an exception if it isn't.  If the length is bogus, this should
      * keep us from trying to allocate an immensely large buffer.
      * (It won't help if the length is *valid* but immensely large,
      * but that's another matter.)
-     *
-     * We do that by attempting to fetch the last byte (if the length
-     * isn't 0).
      */
     if (enc_len != 0)
-	tvb_get_guint8(asn1->tvb, eoc - 1);
+    	tvb_ensure_bytes_exist(asn1->tvb, asn1->offset, enc_len);
+
+    eoc = asn1->offset + enc_len;
 
     size = enc_len + 1;
     *oid = g_malloc(size * sizeof(gulong));
diff --git a/epan/tvbuff.c b/epan/tvbuff.c
index 1c0ef6ecfd..a79eb2e30d 100644
--- a/epan/tvbuff.c
+++ b/epan/tvbuff.c
@@ -9,7 +9,7 @@
  * 		the data of a backing tvbuff, or can be a composite of
  * 		other tvbuffs.
  *
- * $Id: tvbuff.c,v 1.36 2002/05/05 21:07:52 guy Exp $
+ * $Id: tvbuff.c,v 1.37 2002/05/13 01:24:47 guy Exp $
  *
  * Copyright (c) 2000 by Gilbert Ramirez <gram@alumni.rice.edu>
  *
@@ -421,16 +421,38 @@ static gboolean
 check_offset_length_no_exception(tvbuff_t *tvb, gint offset, gint length,
 		guint *offset_ptr, guint *length_ptr, int *exception)
 {
+	guint	end_offset;
+
 	g_assert(tvb->initialized);
 
 	if (!compute_offset_length(tvb, offset, length, offset_ptr, length_ptr, exception)) {
 		return FALSE;
 	}
 
-	if (*offset_ptr + *length_ptr <= tvb->length) {
+	/*
+	 * Compute the offset of the first byte past the length.
+	 */
+	end_offset = *offset_ptr + *length_ptr;
+
+	/*
+	 * Check for an overflow, and clamp "end_offset" at the maximum
+	 * if we got an overflow - that should force us to indicate that
+	 * we're past the end of the tvbuff.
+	 */
+	if (end_offset < *offset_ptr)
+		end_offset = UINT_MAX;
+
+	/*
+	 * Check whether that offset goes more than one byte past the
+	 * end of the buffer.
+	 *
+	 * If not, return TRUE; otherwise, return FALSE and, if "exception"
+	 * is non-null, return the appropriate exception through it.
+	 */
+	if (end_offset <= tvb->length) {
 		return TRUE;
 	}
-	else if (*offset_ptr + *length_ptr <= tvb->reported_length) {
+	else if (end_offset <= tvb->reported_length) {
 		if (exception) {
 			*exception = BoundsError;
 		}
@@ -446,7 +468,7 @@ check_offset_length_no_exception(tvbuff_t *tvb, gint offset, gint length,
 	g_assert_not_reached();
 }
 
-/* Checks (+/-) offset and length and throws BoundsError if
+/* Checks (+/-) offset and length and throws an exception if
  * either is out of bounds. Sets integer ptrs to the new offset
  * and length. */
 static void
@@ -625,7 +647,7 @@ tvb_ensure_length_remaining(tvbuff_t *tvb, gint offset)
 
 
 /* Validates that 'length' bytes are available starting from
- * offset (pos/neg). Does not throw BoundsError exception. */
+ * offset (pos/neg). Does not throw an exception. */
 gboolean
 tvb_bytes_exist(tvbuff_t *tvb, gint offset, gint length)
 {
@@ -644,6 +666,31 @@ tvb_bytes_exist(tvbuff_t *tvb, gint offset, gint length)
 	}
 }
 
+/* Validates that 'length' bytes are available starting from
+ * offset (pos/neg). Throws an exception if they aren't. */
+void
+tvb_ensure_bytes_exist(tvbuff_t *tvb, gint offset, gint length)
+{
+	guint		abs_offset, abs_length;
+
+	g_assert(tvb->initialized);
+
+	/*
+	 * -1 doesn't mean "until end of buffer", as that's pointless
+	 * for this routine.  We must treat it as a Really Large Positive
+	 * Number, so that we throw an exception; we throw
+	 * ReportedBoundsError, as if it were past even the end of a
+	 * reassembled packet, and past the end of even the data we
+	 * didn't capture.
+	 *
+	 * We do the same with other negative lengths.
+	 */
+	if (length < 0) {
+		THROW(ReportedBoundsError);
+	}
+	check_offset_length(tvb, offset, length, &abs_offset, &abs_length);
+}
+
 gboolean
 tvb_offset_exists(tvbuff_t *tvb, gint offset)
 {
@@ -950,6 +997,17 @@ tvb_memcpy(tvbuff_t *tvb, guint8* target, gint offset, gint length)
 }
 
 
+/*
+ * XXX - this doesn't treat a length of -1 as an error.
+ * If it did, this could replace some code that calls
+ * "tvb_ensure_bytes_exist()" and then allocates a buffer and copies
+ * data to it.
+ *
+ * Does anything depend on this routine treating -1 as meaning "to the
+ * end of the buffer"?  If so, perhaps we need two routines, one that
+ * treats -1 as such, and one that checks for -1 and then calls this
+ * routine.
+ */
 guint8*
 tvb_memdup(tvbuff_t *tvb, gint offset, gint length)
 {
diff --git a/epan/tvbuff.h b/epan/tvbuff.h
index 159c4fa4ff..4b96f2a519 100644
--- a/epan/tvbuff.h
+++ b/epan/tvbuff.h
@@ -9,7 +9,7 @@
  * 		the data of a backing tvbuff, or can be a composite of
  * 		other tvbuffs.
  *
- * $Id: tvbuff.h,v 1.26 2002/05/05 00:57:59 guy Exp $
+ * $Id: tvbuff.h,v 1.27 2002/05/13 01:24:47 guy Exp $
  *
  * Copyright (c) 2000 by Gilbert Ramirez <gram@alumni.rice.edu>
  *
@@ -192,13 +192,17 @@ extern guint tvb_length(tvbuff_t*);
  * indicate that offset is out of bounds. No exception is thrown. */
 extern gint tvb_length_remaining(tvbuff_t*, gint offset);
 
-/* Same as above, but throws BoundsError if the offset is out of bounds. */
+/* Same as above, but throws an exception if the offset is out of bounds. */
 extern guint tvb_ensure_length_remaining(tvbuff_t*, gint offset);
 
 /* Checks (w/o throwing exception) that the bytes referred to by
  * 'offset'/'length' actually exist in the buffer */
 extern gboolean tvb_bytes_exist(tvbuff_t*, gint offset, gint length);
 
+/* Checks that the bytes referred to by 'offset'/'length' actually exist
+ * in the buffer, and throws an exception if they aren't. */
+extern void tvb_ensure_bytes_exist(tvbuff_t *tvb, gint offset, gint length);
+
 /* Checks (w/o throwing exception) that offset exists in buffer */
 extern gboolean tvb_offset_exists(tvbuff_t*, gint offset);
 
@@ -223,7 +227,7 @@ extern void tvb_set_reported_length(tvbuff_t*, guint);
 extern gint tvb_raw_offset(tvbuff_t*);
 
 /************** START OF ACCESSORS ****************/
-/* All accessors will throw BoundsError or ReportedBoundsError if appropriate */
+/* All accessors will throw an exception if appropriate */
 
 extern guint8  tvb_get_guint8(tvbuff_t*, gint offset);
 
diff --git a/packet-giop.c b/packet-giop.c
index 8a8e12e3cb..09959b6853 100644
--- a/packet-giop.c
+++ b/packet-giop.c
@@ -9,7 +9,7 @@
  * Frank Singleton <frank.singleton@ericsson.com>
  * Trevor Shepherd <eustrsd@am1.ericsson.se>
  *
- * $Id: packet-giop.c,v 1.60 2002/05/12 20:43:29 gerald Exp $
+ * $Id: packet-giop.c,v 1.61 2002/05/13 01:24:45 guy Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@ethereal.com>
@@ -2327,17 +2327,15 @@ guint8 get_CDR_octet(tvbuff_t *tvb, int *offset) {
 
 void get_CDR_octet_seq(tvbuff_t *tvb, gchar **seq, int *offset, guint32 len) {
 
-  guint32 remain_len = tvb_length_remaining(tvb, *offset);
-
-  if (remain_len < len) {
-    /*
-     * Generate an exception, and stop processing.
-     * We do that now, rather than after allocating the buffer, so we
-     * don't have to worry about freeing the buffer.
-     * XXX - would we be better off using a cleanup routine?
-     */
-    tvb_ensure_length_remaining(tvb, *offset + remain_len + 1);
-  }
+  /*
+   * Make sure that the entire sequence of octets is in the buffer before
+   * allocating the buffer, so that we don't have to worry about freeing
+   * the buffer, and so that we don't try to allocate a buffer bigger
+   * than the data we'll actually be copying, and thus don't run the risk
+   * of crashing if the buffer is *so* big that we fail to allocate it
+   * and "g_new0()" aborts.
+   */
+  tvb_ensure_bytes_exist(tvb, *offset, len);
 
   /*
    * XXX - should we just allocate "len" bytes, and have "get_CDR_string()"
@@ -2639,9 +2637,9 @@ guint16 get_CDR_ushort(tvbuff_t *tvb, int *offset, gboolean stream_is_big_endian
  * Wchar is not supported for GIOP 1.0.
  */
 
-gint8 get_CDR_wchar(tvbuff_t *tvb, gchar **seq, int *offset, MessageHeader * header) {
+gint get_CDR_wchar(tvbuff_t *tvb, gchar **seq, int *offset, MessageHeader * header) {
   
-  guint32 slength;
+  gint slength;
   gchar *raw_wstring;
 
   /* CORBA chapter 15:
diff --git a/packet-giop.h b/packet-giop.h
index 690f796176..3e43eac4dc 100644
--- a/packet-giop.h
+++ b/packet-giop.h
@@ -4,7 +4,7 @@
  *
  * Based on CORBAv2.4.2  Chapter 15 GIOP Description.
  *
- * $Id: packet-giop.h,v 1.8 2002/05/12 20:43:29 gerald Exp $
+ * $Id: packet-giop.h,v 1.9 2002/05/13 01:24:46 guy Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@ethereal.com>
@@ -484,7 +484,7 @@ extern guint16 get_CDR_ushort(tvbuff_t *tvb, int *offset,
  * Wchar is not supported for GIOP 1.0.
  */
 
-extern gint8 get_CDR_wchar(tvbuff_t *tvb, gchar **seq, int *offset,
+extern gint get_CDR_wchar(tvbuff_t *tvb, gchar **seq, int *offset,
     MessageHeader * header);