When we passed the crytobuffer to krb5_c_decrypt() we never actually

verified that we did have enough data in the buffer/tvb, which could
lead to a SEGV.
(for example if we enable KRB5 decryption but we do NOT use TCP
reassembly, and the encrypted data goes beyong the end of the current
segment)

Change the signature to decrypt_krb5_data() to take a TVB instead of a
buffer+length.
Actually check that we do have the entire encrypted PDU before calling
out to the kerberos libraries.

git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@29213 f5534014-38df-0310-8fa8-9805f1628bb7

diff --git a/asn1/spnego/packet-spnego-template.c b/asn1/spnego/packet-spnego-template.c
index c9029979c212b9afbeb98b4e313cb38a7e22b52d..7552cf2c4c188d4cc29aef38c25ede074ea5a436 100644 (file)
--- a/asn1/spnego/packet-spnego-template.c
+++ b/asn1/spnego/packet-spnego-template.c
@@ -698,6 +698,7 @@ decrypt_gssapi_krb_cfx_wrap(proto_tree *tree _U_, packet_info *pinfo _U_, tvbuff
        char *rotated;
        char *output;
        int datalen;
+       tvbuff_t *next_tvb;
 
        /* dont do anything if we are not attempting to decrypt data */
        if(!krb_decrypt){
@@ -709,8 +710,11 @@ decrypt_gssapi_krb_cfx_wrap(proto_tree *tree _U_, packet_info *pinfo _U_, tvbuff
        tvb_memcpy(tvb, rotated, 0, tvb_length(tvb));
        res = rrc_rotate(rotated, tvb_length(tvb), rrc, TRUE);
 
-       output = decrypt_krb5_data(tree, pinfo, usage, tvb_length(tvb),
-                 rotated, keytype, &datalen);
+       next_tvb=tvb_new_child_real_data(tvb, rotated, tvb_length(tvb), tvb_reported_length(tvb));
+       add_new_data_source(pinfo, next_tvb, "GSSAPI CFX");
+
+       output = decrypt_krb5_data(tree, pinfo, usage, next_tvb,
+                 keytype, &datalen);
 
        if (output) {
                char *outdata;

diff --git a/epan/dissectors/packet-kerberos.c b/epan/dissectors/packet-kerberos.c
index 850a24b5f6b41efe445284348d5ac4e384d0061a..c8ac22589170bd4e73b37b1a921b6c2780518eab 100644 (file)
--- a/epan/dissectors/packet-kerberos.c
+++ b/epan/dissectors/packet-kerberos.c
@@ -501,8 +501,7 @@ read_keytab_file(const char *filename)
 guint8 *
 decrypt_krb5_data(proto_tree *tree, packet_info *pinfo,
                        int usage,
-                       int length,
-                       const guint8 *cryptotext,
+                       tvbuff_t *cryptotvb,
                        int keytype,
                        int *datalen)
 {
@@ -511,12 +510,19 @@ decrypt_krb5_data(proto_tree *tree, packet_info *pinfo,
        enc_key_t *ek;
        static krb5_data data = {0,0,NULL};
        krb5_keytab_entry key;
+       int length = tvb_length(cryptotvb);
+       const guint8 *cryptotext = tvb_get_ptr(cryptotvb, 0, length);
 
        /* don't do anything if we are not attempting to decrypt data */
        if(!krb_decrypt){
                return NULL;
        }
 
+       /* make sure we have all the data we need */
+       if (tvb_length(cryptotvb) < tvb_reported_length(cryptotvb)) {
+               return NULL;
+       }
+
        /* XXX we should only do this for first time, then store somewhere */
        /* XXX We also need to re-read the keytab when the preference changes */
 
@@ -636,8 +642,7 @@ read_keytab_file(const char *filename)
 guint8 *
 decrypt_krb5_data(proto_tree *tree, packet_info *pinfo,
                        int usage,
-                       int length,
-                       const guint8 *cryptotext,
+                       tvbuff_t *cryptotvb,
                        int keytype,
                        int *datalen)
 {
@@ -645,12 +650,19 @@ decrypt_krb5_data(proto_tree *tree, packet_info *pinfo,
        krb5_error_code ret;
        krb5_data data;
        enc_key_t *ek;
+       int length = tvb_length(cryptotvb);
+       const guint8 *cryptotext = tvb_get_ptr(cryptotvb, 0, length);
 
        /* don't do anything if we are not attempting to decrypt data */
        if(!krb_decrypt){
                return NULL;
        }
 
+       /* make sure we have all the data we need */
+       if (tvb_length(cryptotvb) < tvb_reported_length(cryptotvb)) {
+               return NULL;
+       }
+
        /* XXX we should only do this for first time, then store somewhere */
        /* XXX We also need to re-read the keytab when the preference changes */
 
@@ -813,8 +825,7 @@ g_warning("added key: %s", sk->origin);
 guint8 *
 decrypt_krb5_data(proto_tree *tree, packet_info *pinfo,
                        int _U_ usage,
-                       int length,
-                       const guint8 *cryptotext,
+                       tvbuff_t *cryptotvb,
                        int keytype,
                        int *datalen)
 {
@@ -835,6 +846,8 @@ decrypt_krb5_data(proto_tree *tree, packet_info *pinfo,
        GSList *ske;
        service_key_t *sk;
        struct des3_ctx ctx;
+       int length = tvb_length(cryptotvb);
+       const guint8 *cryptotext = tvb_get_ptr(cryptotvb, 0, length);
 
 
        /* don't do anything if we are not attempting to decrypt data */
@@ -842,6 +855,11 @@ decrypt_krb5_data(proto_tree *tree, packet_info *pinfo,
                return NULL;
        }
 
+       /* make sure we have all the data we need */
+       if (tvb_length(cryptotvb) < tvb_reported_length(cryptotvb)) {
+               return NULL;
+       }
+
        if (keytype != KEYTYPE_DES3_CBC_MD5 || service_key_list == NULL) {
                return NULL;
        }
@@ -2105,7 +2123,10 @@ dissect_krb5_decrypt_PA_ENC_TIMESTAMP (proto_tree *tree, tvbuff_t *tvb, int offs
         * == 1
         */
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 1, length, tvb_get_ptr(tvb, offset, length), PA_ENC_TIMESTAMP_etype, NULL);
+               tvbuff_t *next_tvb;
+
+               next_tvb=tvb_new_subset(tvb, offset, tvb_length_remaining(tvb, offset), tvb_reported_length_remaining(tvb, offset));
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 1, next_tvb, PA_ENC_TIMESTAMP_etype, NULL);
        }
 
        if(plaintext){
@@ -3490,7 +3511,10 @@ dissect_krb5_decrypt_PRIV (proto_tree *tree, tvbuff_t *tvb, int offset, asn1_ctx
        length=tvb_length_remaining(tvb, offset);
 
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 13, length, tvb_get_ptr(tvb, offset, length), PRIV_etype, NULL);
+               tvbuff_t *next_tvb;
+
+               next_tvb=tvb_new_subset(tvb, offset, tvb_length_remaining(tvb, offset), tvb_reported_length_remaining(tvb, offset));
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 13, next_tvb, PRIV_etype, NULL);
        }
 
        if(plaintext){
@@ -3635,6 +3659,9 @@ dissect_krb5_decrypt_EncKrbCredPart (proto_tree *tree, tvbuff_t *tvb, int offset
 {
        guint8 *plaintext=NULL;
        int length;
+       tvbuff_t *next_tvb;
+
+       next_tvb=tvb_new_subset(tvb, offset, tvb_length_remaining(tvb, offset), tvb_reported_length_remaining(tvb, offset));
 
        length=tvb_length_remaining(tvb, offset);
 
@@ -3643,7 +3670,7 @@ dissect_krb5_decrypt_EncKrbCredPart (proto_tree *tree, tvbuff_t *tvb, int offset
         * == 14
         */
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 14, length, tvb_get_ptr(tvb, offset, length), EncKrbCredPart_etype, NULL);
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 14, next_tvb, EncKrbCredPart_etype, NULL);
        }
 
        if(plaintext){
@@ -3789,6 +3816,9 @@ dissect_krb5_decrypt_enc_authorization_data(proto_tree *tree, tvbuff_t *tvb, int
 {
        guint8 *plaintext=NULL;
        int length;
+       tvbuff_t *next_tvb;
+
+       next_tvb=tvb_new_subset(tvb, offset, tvb_length_remaining(tvb, offset), tvb_reported_length_remaining(tvb, offset));
 
        length=tvb_length_remaining(tvb, offset);
 
@@ -3798,10 +3828,10 @@ dissect_krb5_decrypt_enc_authorization_data(proto_tree *tree, tvbuff_t *tvb, int
        if a sub-session key is used, or 4 if the session key is used.
        */
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 4, length, tvb_get_ptr(tvb, offset, length), enc_authorization_data_etype, NULL);
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 4, next_tvb, enc_authorization_data_etype, NULL);
        }
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 5, length, tvb_get_ptr(tvb, offset, length), enc_authorization_data_etype, NULL);
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 5, next_tvb, enc_authorization_data_etype, NULL);
        }
 
        if(plaintext){
@@ -3981,6 +4011,9 @@ dissect_krb5_decrypt_authenticator_data (proto_tree *tree, tvbuff_t *tvb, int of
 {
        guint8 *plaintext=NULL;
        int length;
+       tvbuff_t *next_tvb;
+
+       next_tvb=tvb_new_subset(tvb, offset, tvb_length_remaining(tvb, offset), tvb_reported_length_remaining(tvb, offset));
 
        length=tvb_length_remaining(tvb, offset);
 
@@ -3991,10 +4024,10 @@ dissect_krb5_decrypt_authenticator_data (proto_tree *tree, tvbuff_t *tvb, int of
         * == 11
         */
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 7, length, tvb_get_ptr(tvb, offset, length), authenticator_etype, NULL);
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 7, next_tvb, authenticator_etype, NULL);
        }
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 11, length, tvb_get_ptr(tvb, offset, length), authenticator_etype, NULL);
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 11, next_tvb, authenticator_etype, NULL);
        }
 
        if(plaintext){
@@ -4067,6 +4100,9 @@ dissect_krb5_decrypt_Ticket_data (proto_tree *tree, tvbuff_t *tvb, int offset, a
 {
        guint8 *plaintext;
        int length;
+       tvbuff_t *next_tvb;
+
+       next_tvb=tvb_new_subset(tvb, offset, tvb_length_remaining(tvb, offset), tvb_reported_length_remaining(tvb, offset));
 
        length=tvb_length_remaining(tvb, offset);
 
@@ -4074,7 +4110,7 @@ dissect_krb5_decrypt_Ticket_data (proto_tree *tree, tvbuff_t *tvb, int offset, a
         * 7.5.1
         * All Ticket encrypted parts use usage == 2
         */
-       if( (plaintext=decrypt_krb5_data(tree, actx->pinfo, 2, length, tvb_get_ptr(tvb, offset, length), Ticket_etype, NULL)) ){
+       if( (plaintext=decrypt_krb5_data(tree, actx->pinfo, 2, next_tvb, Ticket_etype, NULL)) ){
                tvbuff_t *next_tvb;
                next_tvb = tvb_new_child_real_data(tvb, plaintext,
                                           length,
@@ -4205,7 +4241,10 @@ dissect_krb5_decrypt_AP_REP_data(proto_tree *tree, tvbuff_t *tvb, int offset, as
         * == 11
         */
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 12, length, tvb_get_ptr(tvb, offset, length), AP_REP_etype, NULL);
+               tvbuff_t *next_tvb;
+
+               next_tvb=tvb_new_subset(tvb, offset, tvb_length_remaining(tvb, offset), tvb_reported_length_remaining(tvb, offset));
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 12, next_tvb, AP_REP_etype, NULL);
        }
 
        if(plaintext){
@@ -4301,6 +4340,9 @@ dissect_krb5_decrypt_KDC_REP_data (proto_tree *tree, tvbuff_t *tvb, int offset,
 {
        guint8 *plaintext=NULL;
        int length;
+       tvbuff_t *next_tvb;
+
+       next_tvb=tvb_new_subset(tvb, offset, tvb_length_remaining(tvb, offset), tvb_reported_length_remaining(tvb, offset));
 
        length=tvb_length_remaining(tvb, offset);
 
@@ -4312,13 +4354,13 @@ dissect_krb5_decrypt_KDC_REP_data (proto_tree *tree, tvbuff_t *tvb, int offset,
      * == 9
         */
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 3, length, tvb_get_ptr(tvb, offset, length), KDC_REP_etype, NULL);
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 3, next_tvb, KDC_REP_etype, NULL);
        }
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 8, length, tvb_get_ptr(tvb, offset, length), KDC_REP_etype, NULL);
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 8, next_tvb, KDC_REP_etype, NULL);
        }
        if(!plaintext){
-               plaintext=decrypt_krb5_data(tree, actx->pinfo, 9, length, tvb_get_ptr(tvb, offset, length), KDC_REP_etype, NULL);
+               plaintext=decrypt_krb5_data(tree, actx->pinfo, 9, next_tvb, KDC_REP_etype, NULL);
        }
 
        if(plaintext){

diff --git a/epan/dissectors/packet-kerberos.h b/epan/dissectors/packet-kerberos.h
index c9607bd7a6d844adbb3e866f80b3745f3b766243..7c6115367956768c27e1b7aae88a248b5cdc804e 100644 (file)
--- a/epan/dissectors/packet-kerberos.h
+++ b/epan/dissectors/packet-kerberos.h
@@ -77,8 +77,7 @@ extern enc_key_t *enc_key_list;
 guint8 *
 decrypt_krb5_data(proto_tree *tree, packet_info *pinfo,
                        int usage,
-                       int length,
-                       const guint8 *cryptotext,
+                       tvbuff_t *crypototvb,
                        int keytype,
                        int *datalen);
 

diff --git a/epan/dissectors/packet-kink.c b/epan/dissectors/packet-kink.c
index 4c108d7482b7bba04a478ea1bf7b7875a4f53398..440fa2ff0b328aa383723dbb567f467345523c69 100644 (file)
--- a/epan/dissectors/packet-kink.c
+++ b/epan/dissectors/packet-kink.c
@@ -703,16 +703,12 @@ dissect_payload_kink_encrypt(packet_info *pinfo, tvbuff_t *tvb, int offset, prot
   proto_item *ti;
   guint8 next_payload;
   guint8 reserved;
-  guint payload_length,encrypt_length;
+  guint payload_length;
+  gint encrypt_length;
   guint8 inner_next_pload;
   guint32 reserved2;
   guint16 inner_payload_length;
   int start_payload_offset = 0;    /* Keep the begining of the payload offset */
-  const guint8 *data_value;
-#ifdef HAVE_KERBEROS
-  tvbuff_t *next_tvb;
-  guint8 *plaintext=NULL;
-#endif
 
   payload_length = tvb_get_ntohs(tvb,offset + TO_PAYLOAD_LENGTH);
   start_payload_offset = offset;
@@ -739,13 +735,15 @@ dissect_payload_kink_encrypt(packet_info *pinfo, tvbuff_t *tvb, int offset, prot
   }
   offset += 2;
 
-  data_value = tvb_get_ptr(tvb, offset, encrypt_length);
-
   /* decrypt kink encrypt */
 
   if(keytype != 0){
 #ifdef HAVE_KERBEROS
-    plaintext=decrypt_krb5_data(tree, pinfo, 0, encrypt_length, data_value, keytype, NULL);    
+    tvbuff_t *next_tvb;
+    guint8 *plaintext=NULL;
+
+    next_tvb=tvb_new_subset(tvb, offset, MIN(tvb_length_remaining(tvb, offset), encrypt_length), encrypt_length);
+    plaintext=decrypt_krb5_data(tree, pinfo, 0, next_tvb, keytype, NULL);    
     if(plaintext){
       next_tvb=tvb_new_child_real_data(tvb, plaintext, encrypt_length, encrypt_length);
       add_new_data_source(pinfo, next_tvb, "decrypted kink encrypt");

diff --git a/epan/dissectors/packet-spnego.c b/epan/dissectors/packet-spnego.c
index f9b10a418f2551d7a751aa34b49e69e3160eea8f..82d8d8e58b279a5eea289b52aef4d1c7c0e97773 100644 (file)
--- a/epan/dissectors/packet-spnego.c
+++ b/epan/dissectors/packet-spnego.c
@@ -1197,6 +1197,7 @@ decrypt_gssapi_krb_cfx_wrap(proto_tree *tree _U_, packet_info *pinfo _U_, tvbuff
        char *rotated;
        char *output;
        int datalen;
+       tvbuff_t *next_tvb;
 
        /* dont do anything if we are not attempting to decrypt data */
        if(!krb_decrypt){
@@ -1208,8 +1209,11 @@ decrypt_gssapi_krb_cfx_wrap(proto_tree *tree _U_, packet_info *pinfo _U_, tvbuff
        tvb_memcpy(tvb, rotated, 0, tvb_length(tvb));
        res = rrc_rotate(rotated, tvb_length(tvb), rrc, TRUE);
 
-       output = decrypt_krb5_data(tree, pinfo, usage, tvb_length(tvb),
-                 rotated, keytype, &datalen);
+       next_tvb=tvb_new_child_real_data(tvb, rotated, tvb_length(tvb), tvb_reported_length(tvb));
+       add_new_data_source(pinfo, next_tvb, "GSSAPI CFX");
+
+       output = decrypt_krb5_data(tree, pinfo, usage, next_tvb,
+                 keytype, &datalen);
 
        if (output) {
                char *outdata;
@@ -1959,7 +1963,7 @@ void proto_register_spnego(void) {
         NULL, HFILL }},
 
 /*--- End of included file: packet-spnego-hfarr.c ---*/
-#line 1375 "packet-spnego-template.c"
+#line 1379 "packet-spnego-template.c"
        };
 
        /* List of subtrees */
@@ -1981,7 +1985,7 @@ void proto_register_spnego(void) {
     &ett_spnego_InitialContextToken_U,
 
 /*--- End of included file: packet-spnego-ettarr.c ---*/
-#line 1385 "packet-spnego-template.c"
+#line 1389 "packet-spnego-template.c"
        };
 
        /* Register protocol */