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.



svn path=/trunk/; revision=29213

5 files changed, 82 insertions, 35 deletions

diff --git a/asn1/spnego/packet-spnego-template.c b/asn1/spnego/packet-spnego-template.c
index c9029979c2..7552cf2c4c 100644
--- 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 850a24b5f6..c8ac225891 100644
--- 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 c9607bd7a6..7c61153679 100644
--- 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 4c108d7482..440fa2ff0b 100644
--- 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 f9b10a418f..82d8d8e58b 100644
--- 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 */