alljoyn: fix signature length adjustments

Ensure that the signature pointer and length always matches, otherwise a
buffer overrun (read) is possible.

Tested with the original captures from bug 12953, the PDML output is
still the same while the fuzzed capture does not crash anymore.

Bug: 12953
Change-Id: I8843a5daf98a79fb19906e824326cdf619164484
Reviewed-on: https://code.wireshark.org/review/18347
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>

1 files changed, 18 insertions, 13 deletions

diff --git a/epan/dissectors/packet-alljoyn.c b/epan/dissectors/packet-alljoyn.c
index fd9b1dd93e..76dccb9ce1 100644
--- a/epan/dissectors/packet-alljoyn.c
+++ b/epan/dissectors/packet-alljoyn.c
@@ -759,8 +759,9 @@ advance_to_end_of_signature(const guint8 **signature,
     gint8 current_type;
     gint8 end_type = ARG_INVALID;
 
-    while(*(++(*signature)) && --(*signature_length) > 0 && !done) {
-        current_type = **signature;
+    while (*signature_length > 0 && **signature && !done) {
+        current_type = *(++(*signature));
+        --*signature_length;
 
         /* Were we looking for the end of a structure or dictionary? If so, did we find it? */
         if(end_type != ARG_INVALID) {
@@ -892,7 +893,6 @@ parse_arg(tvbuff_t      *tvb,
             const guint8 *sig_saved;
             gint          starting_offset;
             gint          number_of_items      = 0;
-            guint8        remaining_sig_length = *signature_length;
             gint          packet_length        = (gint)tvb_reported_length(tvb);
 
             header_type_name = "array";
@@ -928,14 +928,17 @@ parse_arg(tvbuff_t      *tvb,
             add_padding_item(padding_start, offset, tvb, tree);
 
             if(0 == length) {
-                advance_to_end_of_signature(signature, &remaining_sig_length);
+                advance_to_end_of_signature(signature, signature_length);
             } else {
+                guint8 sig_length_saved = *signature_length - 1;
+
                 while((offset - starting_offset) < length) {
                     const guint8 *sig_pointer;
+                    guint8        remaining_sig_length;
 
                     number_of_items++;
                     sig_pointer = sig_saved;
-                    remaining_sig_length = *signature_length - 1;
+                    remaining_sig_length = sig_length_saved;
 
                     offset = parse_arg(tvb,
                                        pinfo,
@@ -952,11 +955,10 @@ parse_arg(tvbuff_t      *tvb,
 
                     /* Set the signature pointer to be just past the type just handled. */
                     *signature = sig_pointer;
+                    *signature_length = remaining_sig_length;
                 }
             }
 
-            *signature_length = remaining_sig_length;
-
             if(item) {
                 proto_item_append_text(item, " of %d '%c' elements", number_of_items, *sig_saved);
             }
@@ -985,23 +987,26 @@ parse_arg(tvbuff_t      *tvb,
 
     case ARG_SIGNATURE:  /* AllJoyn signature basic type */
         header_type_name  = "signature";
-        *signature_length = tvb_get_guint8(tvb, offset);
+        length = tvb_get_guint8(tvb, offset);
 
-        if(*signature_length + 2 > tvb_reported_length_remaining(tvb, offset)) {
+        if (length + 2 > tvb_reported_length_remaining(tvb, offset)) {
             gint bytes_left = tvb_reported_length_remaining(tvb, offset);
 
             col_add_fstr(pinfo->cinfo, COL_INFO, "BAD DATA: Signature length is %d. Only %d bytes left in packet.",
-                         (gint)(*signature_length), bytes_left);
+                         length, bytes_left);
             return tvb_reported_length(tvb);
         }
 
         /* Include the terminating '/0'. */
-        length = *signature_length + 1;
+        length++;
 
         proto_tree_add_item(field_tree, hf_alljoyn_mess_body_signature_length, tvb, offset, 1, encoding);
         offset += 1;
 
+        /* Extract signature from tvb and return to caller. */
+        /* XXX should this extract "length - 1" since we always expect /0? */
         proto_tree_add_item_ret_string(field_tree, hf_alljoyn_mess_body_signature, tvb, offset, length, ENC_ASCII|ENC_NA, wmem_packet_scope(), signature);
+        *signature_length = length;
 
         if(HDR_SIGNATURE == field_code) {
             col_append_fstr(pinfo->cinfo, COL_INFO, " (%s)", *signature);
@@ -1268,7 +1273,7 @@ parse_arg(tvbuff_t      *tvb,
         break;
     }
 
-    if(*signature && ARG_ARRAY != type_id && HDR_INVALID == field_code) {
+    if (*signature && *signature_length > 0 && ARG_ARRAY != type_id && HDR_INVALID == field_code) {
         (*signature)++;
         (*signature_length)--;
     }
@@ -1453,7 +1458,7 @@ handle_message_body_parameters(tvbuff_t     *tvb,
         end_of_body = packet_length;
     }
 
-    while(offset < end_of_body && signature && *signature) {
+    while(offset < end_of_body && signature_length > 0 && signature && *signature) {
         offset = parse_arg(tvb,
                            pinfo,
                            NULL,