IEEE 802.15.4: fix decryption results based on uninitialized data

Refactor ieee802154_set_mac_key to return the number of keys set and
only try to decrypt with the alt_key if actually provided

Bug: 14522
Change-Id: I40802dff8c08f7f82b792fb16f5f91aa3b9e20cc
Reviewed-on: https://code.wireshark.org/review/26677
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

5 files changed, 30 insertions, 28 deletions

diff --git a/epan/dissectors/packet-ieee802154.c b/epan/dissectors/packet-ieee802154.c
index d74abd34b4..7e7c9ab4aa 100644
--- a/epan/dissectors/packet-ieee802154.c
+++ b/epan/dissectors/packet-ieee802154.c
@@ -282,7 +282,7 @@ static void dissect_ieee802154_gtsreq          (tvbuff_t *, packet_info *, proto
 /* Decryption helpers. */
 static tvbuff_t *dissect_ieee802154_decrypt(tvbuff_t *, guint, packet_info *, ieee802154_packet *, ieee802154_decrypt_info_t*);
 
-static gboolean ieee802154_set_mac_key(ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t* uat_key);
+static guint ieee802154_set_mac_key(ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t *uat_key);
 
 /* Initialize Protocol and Registered fields */
 static int proto_ieee802154_nonask_phy = -1;
@@ -996,7 +996,7 @@ dissect_ieee802154_fcf(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, ieee
     *offset += 2;
 } /* dissect_ieee802154_fcf */
 
-void register_ieee802154_mac_key_hash_handler(guint hash_identifier, ieee802154_set_mac_key_func key_func)
+void register_ieee802154_mac_key_hash_handler(guint hash_identifier, ieee802154_set_key_func key_func)
 {
     /* Ensure no duplication */
     DISSECTOR_ASSERT(wmem_tree_lookup32(mac_key_hash_handlers, hash_identifier) == NULL);
@@ -1086,14 +1086,17 @@ tvbuff_t *decrypt_ieee802154_payload(tvbuff_t * tvb, guint offset, packet_info *
 
     /* Lookup the key. */
     for (decrypt_info->key_number = 0; decrypt_info->key_number < num_ieee802154_keys; decrypt_info->key_number++) {
-        if (set_key_func(packet, key, alt_key, &ieee802154_keys[decrypt_info->key_number])) {
+        guint nkeys = set_key_func(packet, key, alt_key, &ieee802154_keys[decrypt_info->key_number]);
+        if (nkeys >= 1) {
             /* Try with the initial key */
             decrypt_info->key = key;
             payload_tvb = decrypt_func(tvb, offset, pinfo, packet, decrypt_info);
             if (!((*decrypt_info->status == DECRYPT_PACKET_MIC_CHECK_FAILED) || (*decrypt_info->status == DECRYPT_PACKET_DECRYPT_FAILED))) {
                 break;
             }
-            /* Try with the alternate key */
+        }
+        if (nkeys >= 2) {
+            /* Try also with the alternate key */
             decrypt_info->key = alt_key;
             payload_tvb = decrypt_func(tvb, offset, pinfo, packet, decrypt_info);
             if (!((*decrypt_info->status == DECRYPT_PACKET_MIC_CHECK_FAILED) || (*decrypt_info->status == DECRYPT_PACKET_DECRYPT_FAILED))) {
@@ -3982,9 +3985,9 @@ gboolean ieee802154_long_addr_equal(gconstpointer a, gconstpointer b)
 }
 
 /* Set MAC key function. */
-static gboolean ieee802154_set_mac_key(ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t* uat_key)
+static guint ieee802154_set_mac_key(ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t *uat_key)
 {
-    ieee802154_set_mac_key_func func = (ieee802154_set_mac_key_func)wmem_tree_lookup32(mac_key_hash_handlers, uat_key->hash_type);
+    ieee802154_set_key_func func = (ieee802154_set_key_func)wmem_tree_lookup32(mac_key_hash_handlers, uat_key->hash_type);
 
     if (func != NULL)
         return func(packet, key, alt_key, uat_key);
@@ -3994,10 +3997,10 @@ static gboolean ieee802154_set_mac_key(ieee802154_packet *packet, unsigned char
     if (packet->key_index == uat_key->key_index)
     {
         memcpy(key, uat_key->key, IEEE802154_CIPHER_SIZE);
-        return TRUE;
+        return 1;
     }
 
-    return FALSE;
+    return 0;
 }
 
 /**
diff --git a/epan/dissectors/packet-ieee802154.h b/epan/dissectors/packet-ieee802154.h
index 43be02d328..5d828c370f 100644
--- a/epan/dissectors/packet-ieee802154.h
+++ b/epan/dissectors/packet-ieee802154.h
@@ -542,19 +542,19 @@ typedef struct {
     ieee802154_decrypt_status* status;
 } ieee802154_decrypt_info_t;
 
-/** Fill key and alt_key based on the provided information from the frame and an IEEE 802.15.4 preference table entry */
-typedef gboolean (*ieee802154_set_key_func) (ieee802154_packet * packet, unsigned char* key, unsigned char* alt_key, ieee802154_key_t* key_info);
+/** Fill key and alt_key based on the provided information from the frame and an IEEE 802.15.4 preference table entry
+ * and return the number of keys set (0: none, 1: just key, 2: key and alt_key) */
+typedef guint (*ieee802154_set_key_func) (ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t *uat_key);
 /** Decrypt the payload with the provided information */
 typedef tvbuff_t* (*ieee802154_decrypt_func) (tvbuff_t *, guint, packet_info *, ieee802154_packet *, ieee802154_decrypt_info_t*);
 /** Loop over the keys specified in the IEEE 802.15.4 preferences, try to use them with the specified set_key_func
  * and try to decrypt with the specified decrypt_func
  */
-tvbuff_t *decrypt_ieee802154_payload(tvbuff_t * tvb, guint offset, packet_info * pinfo, proto_tree* key_tree, ieee802154_packet * packet,
-                                     ieee802154_decrypt_info_t* decrypt_info, ieee802154_set_key_func set_key_func, ieee802154_decrypt_func decrypt_func);
+tvbuff_t *decrypt_ieee802154_payload(tvbuff_t *tvb, guint offset, packet_info *pinfo, proto_tree *key_tree, ieee802154_packet *packet,
+                                     ieee802154_decrypt_info_t *decrypt_info, ieee802154_set_key_func set_key_func, ieee802154_decrypt_func decrypt_func);
 
 
-typedef gboolean (*ieee802154_set_mac_key_func) (ieee802154_packet * packet, unsigned char* key, unsigned char* alt_key, ieee802154_key_t* uat_key);
-extern void register_ieee802154_mac_key_hash_handler(guint hash_identifier, ieee802154_set_mac_key_func key_func);
+extern void register_ieee802154_mac_key_hash_handler(guint hash_identifier, ieee802154_set_key_func key_func);
 
 /* Short to Extended Address Prototypes */
 extern ieee802154_map_rec *ieee802154_addr_update(ieee802154_map_tab_t *, guint16, guint16, guint64,
diff --git a/epan/dissectors/packet-mle.c b/epan/dissectors/packet-mle.c
index 57c040a2a1..5a2b79715b 100644
--- a/epan/dissectors/packet-mle.c
+++ b/epan/dissectors/packet-mle.c
@@ -497,7 +497,7 @@ dissect_mle_decrypt(tvbuff_t * tvb,
     return ptext_tvb;
 } /* dissect_mle_decrypt */
 
-void register_mle_key_hash_handler(guint hash_identifier, ieee802154_set_mac_key_func key_func)
+void register_mle_key_hash_handler(guint hash_identifier, ieee802154_set_key_func key_func)
 {
     /* Ensure no duplication */
     DISSECTOR_ASSERT(wmem_tree_lookup32(mle_key_hash_handlers, hash_identifier) == NULL);
@@ -506,9 +506,9 @@ void register_mle_key_hash_handler(guint hash_identifier, ieee802154_set_mac_key
 }
 
 /* Set MLE key function. */
-static gboolean ieee802154_set_mle_key(ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t* uat_key)
+static guint ieee802154_set_mle_key(ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t *uat_key)
 {
-    mle_set_mle_key_func func = (mle_set_mle_key_func)wmem_tree_lookup32(mle_key_hash_handlers, uat_key->hash_type);
+    ieee802154_set_key_func func = (ieee802154_set_key_func)wmem_tree_lookup32(mle_key_hash_handlers, uat_key->hash_type);
 
     if (func != NULL)
         return func(packet, key, alt_key, uat_key);
@@ -518,10 +518,10 @@ static gboolean ieee802154_set_mle_key(ieee802154_packet *packet, unsigned char
     if (packet->key_index == uat_key->key_index)
     {
         memcpy(key, uat_key->key, IEEE802154_CIPHER_SIZE);
-        return TRUE;
+        return 1;
     }
 
-    return FALSE;
+    return 0;
 }
 
 static int
diff --git a/epan/dissectors/packet-mle.h b/epan/dissectors/packet-mle.h
index 7ff2b0b6f8..17ddf80165 100644
--- a/epan/dissectors/packet-mle.h
+++ b/epan/dissectors/packet-mle.h
@@ -14,7 +14,6 @@
 
 #include "packet-ieee802154.h"
 
-typedef gboolean (*mle_set_mle_key_func) (ieee802154_packet * packet, unsigned char* key, unsigned char* alt_key, ieee802154_key_t* uat_key);
-extern void register_mle_key_hash_handler(guint hash_identifier, mle_set_mle_key_func key_func);
+extern void register_mle_key_hash_handler(guint hash_identifier, ieee802154_set_key_func key_func);
 
 #endif
diff --git a/epan/dissectors/packet-thread.c b/epan/dissectors/packet-thread.c
index 51dc4a0ce7..a944a4e808 100644
--- a/epan/dissectors/packet-thread.c
+++ b/epan/dissectors/packet-thread.c
@@ -766,7 +766,7 @@ static void create_thread_temp_keys(GByteArray *seq_ctr_bytes, guint16 src_pan,
 }
 
 /* Set MAC key for Thread hash */
-static gboolean set_thread_mac_key(ieee802154_packet * packet, unsigned char* key, unsigned char* alt_key, ieee802154_key_t* uat_key)
+static guint set_thread_mac_key(ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t *uat_key)
 {
     GByteArray *seq_ctr_bytes = NULL;
 
@@ -778,7 +778,7 @@ static gboolean set_thread_mac_key(ieee802154_packet * packet, unsigned char* ke
     {
         /* This is the well-known Thread key. No need for an alternative key */
         memcpy(key, thread_well_known_key, IEEE802154_CIPHER_SIZE);
-        return TRUE;
+        return 1;
     }
     if (seq_ctr_bytes != NULL) {
         create_thread_temp_keys(seq_ctr_bytes, packet->src_pan, uat_key, key, NULL);
@@ -786,14 +786,14 @@ static gboolean set_thread_mac_key(ieee802154_packet * packet, unsigned char* ke
         seq_ctr_bytes->data[3] ^= 0x80;
         create_thread_temp_keys(seq_ctr_bytes, packet->src_pan, uat_key, alt_key, NULL);
         g_byte_array_free(seq_ctr_bytes, TRUE);
-        return TRUE;
+        return 2;
     }
 
-    return FALSE;
+    return 0;
 }
 
 /* Set MLE key for Thread hash */
-static gboolean set_thread_mle_key(ieee802154_packet * packet, unsigned char* key, unsigned char* alt_key, ieee802154_key_t* uat_key)
+static guint set_thread_mle_key(ieee802154_packet *packet, unsigned char *key, unsigned char *alt_key, ieee802154_key_t *uat_key)
 {
     GByteArray *seq_ctr_bytes = NULL;
     if (packet->key_id_mode == KEY_ID_MODE_KEY_INDEX) {
@@ -819,10 +819,10 @@ static gboolean set_thread_mle_key(ieee802154_packet * packet, unsigned char* ke
         seq_ctr_bytes->data[3] ^= 0x80;
         create_thread_temp_keys(seq_ctr_bytes, packet->src_pan, uat_key, NULL, alt_key);
         g_byte_array_free(seq_ctr_bytes, TRUE);
-        return TRUE;
+        return 2;
     }
 
-    return FALSE;
+    return 0;
 }
 
 static guint