Don't trust the pointer value in a packet; it could be invalid, and this

could cause an unsigned length value to be reduced by more than its
value, turning it into a very large value.

I couldn't exactly reproduce bug 6833, but it was due to an attempt to
allocate 4294967110 bytes, and this bug caused remaining_len to equal
4294967110, and it would try to create a reassembled packet tvbuff of
that size, so I'm guessing this fixes 6833.

svn path=/trunk/; revision=41001

diff --git a/epan/dissectors/packet-mp2t.c b/epan/dissectors/packet-mp2t.c
index a583754b2c0b96fd04a64b1a200d433362b6f895..1c717242e93e4e959721f43fc7a821fd2eb80d09 100644 (file)
--- a/epan/dissectors/packet-mp2t.c
+++ b/epan/dissectors/packet-mp2t.c
@@ -583,6 +583,7 @@ mp2t_process_fragmented_payload(tvbuff_t *tvb, gint offset, guint remaining_len,
 {
        tvbuff_t *next_tvb;
        guint8 pointer = 0;
+       proto_item *pi;
        guint stuff_len = 0;
        proto_item *si;
        proto_tree *stuff_tree;
@@ -613,10 +614,17 @@ mp2t_process_fragmented_payload(tvbuff_t *tvb, gint offset, guint remaining_len,
        /* PES packet don't have pointer fields, others do */
        if (pusi_flag && pid_analysis->pload_type != pid_pload_pes) {
                pointer = tvb_get_guint8(tvb, offset);
-               proto_tree_add_text(header_tree, tvb, offset, 1,
-               "Pointer: %u", tvb_get_guint8(tvb, offset));
+               pi = proto_tree_add_text(header_tree, tvb, offset, 1,
+                   "Pointer: %u", tvb_get_guint8(tvb, offset));
                offset++;
                remaining_len--;
+               if (pointer > remaining_len) {
+                       /* Bogus pointer */
+                       expert_add_info_format(pinfo, pi, PI_MALFORMED,
+                           PI_ERROR,
+                           "Pointer value is too large (> remaining data length %u",
+                           remaining_len);
+               }
 
        }
 
@@ -680,6 +688,16 @@ mp2t_process_fragmented_payload(tvbuff_t *tvb, gint offset, guint remaining_len,
        /* The begining of a new packet is present */
        if (pusi_flag) {
 
+               if (pointer > remaining_len) {
+                       /*
+                        * Quit, so we don't use the bogus pointer value;
+                        * that could cause remaining_len to become
+                        * "negative", meaning it becomes a very large
+                        * positive value.
+                        */
+                       return;
+               }
+
                /* Looks like we already have some stuff in the buffer */
                if (fragmentation) {
                        mp2t_fragment_handle(tvb, offset, pinfo, tree, frag_id, frag_cur_pos,