Fix the very long loop reported in https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8911 :

In parseFields() ensure that we have at least one byte so that callers
don't have to protect against it returning an offset which hasn't incremented.

Remove a couple of now-unnecessary length-remaining (aka "will the offset
move?") checks.

In some other checks, use tvb_ensure_length_remaining() rather than calling
tvb_length_remaining() and (potentially) THROWing an exception.  I'm not sure
if these are really necessary now or not...

svn path=/trunk/; revision=50450

1 files changed, 31 insertions, 48 deletions

diff --git a/epan/dissectors/packet-dis-pdus.c b/epan/dissectors/packet-dis-pdus.c
index 583179e7d4..241de23ef2 100644
--- a/epan/dissectors/packet-dis-pdus.c
+++ b/epan/dissectors/packet-dis-pdus.c
@@ -746,7 +746,10 @@ gint parseFields(tvbuff_t *tvb, proto_tree *tree, gint offset, DIS_ParserNode pa
     guint16 spread_spectrum     = 0;
 
 
-    length = tvb_length_remaining(tvb, offset);
+    /* Get the length while ensuring there's at least one byte for us to
+     * decode (if not, throw an exception so as to prevent very long loops).
+     */
+    length = tvb_ensure_length_remaining(tvb, offset+1);
 
     while ((parserNodes[fieldIndex].fieldType != DIS_FIELDTYPE_END)
             && (length > 0 ) )
@@ -1239,23 +1242,18 @@ gint parseFields(tvbuff_t *tvb, proto_tree *tree, gint offset, DIS_ParserNode pa
 
                 for (i = 0; i < numFixed; ++i)
                 {
+                    proto_item *newSubtree;
+
                     /* is remaining length large enough for another fixed datum (ID & value) */
-                    length = tvb_length_remaining(tvb, offset);
-                    if ( length >= 8  )
-                    {
-                        proto_item *newSubtree;
-                        newField = proto_tree_add_text(tree, tvb, offset, -1, "%s",
-                                                       parserNodes[fieldIndex].fieldLabel);
-                        newSubtree = proto_item_add_subtree(newField, ettFixedData);
-                        offset = parseFields
-                            (tvb, newSubtree, offset,
-                             parserNodes[fieldIndex].children);
-                        proto_item_set_end(newField, tvb, offset);
-                    }
-                    else {
-                        THROW(ReportedBoundsError);
-                        break;
-                    }
+                    /* XXX is this really necessary? */
+                    tvb_ensure_length_remaining(tvb, offset+8);
+
+                    newField = proto_tree_add_text(tree, tvb, offset, -1, "%s",
+                                                   parserNodes[fieldIndex].fieldLabel);
+                    newSubtree = proto_item_add_subtree(newField, ettFixedData);
+                    offset = parseFields (tvb, newSubtree, offset,
+                         parserNodes[fieldIndex].children);
+                    proto_item_set_end(newField, tvb, offset);
                 }
             }
             break;
@@ -1277,16 +1275,10 @@ gint parseFields(tvbuff_t *tvb, proto_tree *tree, gint offset, DIS_ParserNode pa
                 for (i = 0; i < numFixed; ++i)
                 {
                     /* is remaining length large enough for another fixed datum ID (32 bit int) */
-                    if (tvb_length_remaining(tvb, offset) >= 4  )
-                    {
-                       offset = parseFields
-                           (tvb, newSubtree, offset,
-                            parserNodes[fieldIndex].children);
-                    }
-                    else {
-                        THROW(ReportedBoundsError);
-                        break;
-                    }
+                    /* XXX is this really necessary? */
+                    tvb_ensure_length_remaining(tvb, offset+4);
+                    offset = parseFields (tvb, newSubtree, offset,
+                        parserNodes[fieldIndex].children);
                 }
                 proto_item_set_end(newField, tvb, offset);
             }
@@ -1374,26 +1366,17 @@ gint parseFields(tvbuff_t *tvb, proto_tree *tree, gint offset, DIS_ParserNode pa
 
                 for (i = 0; i < numVariable; ++i)
                 {
-                    /* simple check to detect malformed, field parsers will detect specifics */
-                    length = tvb_length_remaining(tvb, offset);
-                    if ( length > 0  )
-                    {
-                        proto_item *newSubtree;
-                        newField = proto_tree_add_text(tree, tvb, offset, -1, "%s",
-                                                       parserNodes[fieldIndex].fieldLabel);
-                        newSubtree = proto_item_add_subtree(newField,
-                                                            ettVariableRecords[i]);
-                        offset = parseFields
-                            (tvb, newSubtree, offset,
-                             parserNodes[fieldIndex].children);
-                        offset = parseField_VariableRecord
-                            (tvb, newSubtree, offset);
-                        proto_item_set_end(newField, tvb, offset);
-                    }
-                    else {
-                        THROW(ReportedBoundsError);
-                        break;
-                    }
+                    proto_item *newSubtree;
+                    newField = proto_tree_add_text(tree, tvb, offset, -1, "%s",
+                                                   parserNodes[fieldIndex].fieldLabel);
+                    newSubtree = proto_item_add_subtree(newField,
+                                                        ettVariableRecords[i]);
+                    offset = parseFields
+                        (tvb, newSubtree, offset,
+                         parserNodes[fieldIndex].children);
+                    offset = parseField_VariableRecord
+                        (tvb, newSubtree, offset);
+                    proto_item_set_end(newField, tvb, offset);
                 }
             }
             break;
@@ -1552,7 +1535,7 @@ gint parseFields(tvbuff_t *tvb, proto_tree *tree, gint offset, DIS_ParserNode pa
         case DIS_FIELDTYPE_UA_EMITTER_SYSTEMS:
             {
                 guint i;
-                
+
                 if (numUAEmitter > DIS_PDU_MAX_UA_EMITTER_SYSTEMS)
                 {
                     numUAEmitter = DIS_PDU_MAX_UA_EMITTER_SYSTEMS;