From 084ec81befc937eac81157dd3995092e1de7e566 Mon Sep 17 00:00:00 2001 From: Tom Yu Date: Tue, 12 Jan 2010 21:31:59 +0000 Subject: [PATCH] MITKRB5-SA-2009-004 [CVE-2009-4212] integer underflow in AES and RC4 decryption Fix integer underflow in AES and RC4 decryption. [MITKRB5-SA-2009-004, CVE-2009-4212] ticket: 6637 target_version: 1.7.1 version_fixed: 1.7.1 status: resolved git-svn-id: svn://anonsvn.mit.edu/krb5/branches/krb5-1-7@23651 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/crypto/Makefile.in | 11 ++- src/lib/crypto/arcfour/arcfour.c | 6 ++ src/lib/crypto/deps | 10 +++ src/lib/crypto/dk/dk_aead.c | 2 +- src/lib/crypto/dk/dk_decrypt.c | 6 ++ src/lib/crypto/enc_provider/aes.c | 48 +++++++----- src/lib/crypto/old/old_decrypt.c | 4 +- src/lib/crypto/raw/raw_decrypt.c | 2 + src/lib/crypto/t_short.c | 126 ++++++++++++++++++++++++++++++ 9 files changed, 191 insertions(+), 24 deletions(-) create mode 100644 src/lib/crypto/t_short.c diff --git a/src/lib/crypto/Makefile.in b/src/lib/crypto/Makefile.in index 85e32e483..61bbc1f21 100644 --- a/src/lib/crypto/Makefile.in +++ b/src/lib/crypto/Makefile.in @@ -18,6 +18,7 @@ EXTRADEPSRCS=\ $(srcdir)/t_nfold.c \ $(srcdir)/t_cf2.c \ $(srcdir)/t_encrypt.c \ + $(srcdir)/t_short.c \ $(srcdir)/t_prf.c \ $(srcdir)/t_prng.c \ $(srcdir)/t_hmac.c \ @@ -206,7 +207,7 @@ libcrypto.lib: clean-unix:: clean-liblinks clean-libs clean-libobjs -check-unix:: t_nfold t_encrypt t_prf t_prng t_hmac t_pkcs5 t_cf2 +check-unix:: t_nfold t_encrypt t_prf t_prng t_hmac t_pkcs5 t_cf2 t_short $(RUN_SETUP) $(VALGRIND) ./t_nfold $(RUN_SETUP) $(VALGRIND) ./t_encrypt $(RUN_SETUP) $(VALGRIND) ./t_prng <$(srcdir)/t_prng.seed >t_prng.output && \ @@ -216,6 +217,7 @@ check-unix:: t_nfold t_encrypt t_prf t_prng t_hmac t_pkcs5 t_cf2 diff t_prf.output $(srcdir)/t_prf.expected $(RUN_SETUP) $(VALGRIND) ./t_cf2 <$(srcdir)/t_cf2.in >t_cf2.output diff t_cf2.output $(srcdir)/t_cf2.expected + $(RUN_SETUP) $(VALGRIND) ./t_short # $(RUN_SETUP) $(VALGRIND) ./t_pkcs5 @@ -249,10 +251,15 @@ t_cts$(EXEEXT): t_cts.$(OBJEXT) $(CRYPTO_DEPLIB) $(SUPPORT_DEPLIB) $(CC_LINK) -o $@ t_cts.$(OBJEXT) \ $(K5CRYPTO_LIB) $(COM_ERR_LIB) $(SUPPORT_LIB) +t_short$(EXEEXT): t_short.$(OBJEXT) $(CRYPTO_DEPLIB) $(SUPPORT_DEPLIB) + $(CC_LINK) -o $@ t_short.$(OBJEXT) \ + $(K5CRYPTO_LIB) $(COM_ERR_LIB) $(SUPPORT_LIB) + clean:: $(RM) t_nfold.o t_nfold t_encrypt t_encrypt.o t_prng.o t_prng \ - t_hmac.o t_hmac t_pkcs5.o t_pkcs5 pbkdf2.o t_prf t_prf.o t_cf2 t_cf2.o + t_hmac.o t_hmac t_pkcs5.o t_pkcs5 pbkdf2.o t_prf t_prf.o \ + t_cf2 t_cf2.o t_short t_short.o -$(RM) t_prng.output all-windows:: diff --git a/src/lib/crypto/arcfour/arcfour.c b/src/lib/crypto/arcfour/arcfour.c index da4471459..206c1c37f 100644 --- a/src/lib/crypto/arcfour/arcfour.c +++ b/src/lib/crypto/arcfour/arcfour.c @@ -199,6 +199,12 @@ krb5_arcfour_decrypt(const struct krb5_enc_provider *enc, keylength = enc->keylength; hashsize = hash->hashsize; + /* Verify input and output lengths. */ + if (input->length < hashsize + CONFOUNDERLENGTH) + return KRB5_BAD_MSIZE; + if (output->length < input->length - hashsize - CONFOUNDERLENGTH) + return KRB5_BAD_MSIZE; + d1.length=keybytes; d1.data=malloc(d1.length); if (d1.data == NULL) diff --git a/src/lib/crypto/deps b/src/lib/crypto/deps index b53a47d83..2d5b4a332 100644 --- a/src/lib/crypto/deps +++ b/src/lib/crypto/deps @@ -463,6 +463,16 @@ t_encrypt.so t_encrypt.po $(OUTPRE)t_encrypt.$(OBJEXT): \ $(SRCTOP)/include/krb5.h $(SRCTOP)/include/krb5/locate_plugin.h \ $(SRCTOP)/include/krb5/preauth_plugin.h $(SRCTOP)/include/port-sockets.h \ $(SRCTOP)/include/socket-utils.h etypes.h t_encrypt.c +t_short.so t_short.po $(OUTPRE)t_short.$(OBJEXT): $(BUILDTOP)/include/autoconf.h \ + $(BUILDTOP)/include/krb5/krb5.h $(BUILDTOP)/include/osconf.h \ + $(BUILDTOP)/include/profile.h $(COM_ERR_DEPS) $(SRCTOP)/include/k5-buf.h \ + $(SRCTOP)/include/k5-err.h $(SRCTOP)/include/k5-gmt_mktime.h \ + $(SRCTOP)/include/k5-int-pkinit.h $(SRCTOP)/include/k5-int.h \ + $(SRCTOP)/include/k5-platform.h $(SRCTOP)/include/k5-plugin.h \ + $(SRCTOP)/include/k5-thread.h $(SRCTOP)/include/krb5.h \ + $(SRCTOP)/include/krb5/locate_plugin.h $(SRCTOP)/include/krb5/preauth_plugin.h \ + $(SRCTOP)/include/port-sockets.h $(SRCTOP)/include/socket-utils.h \ + t_short.c t_prf.so t_prf.po $(OUTPRE)t_prf.$(OBJEXT): $(BUILDTOP)/include/autoconf.h \ $(BUILDTOP)/include/krb5/krb5.h $(BUILDTOP)/include/osconf.h \ $(BUILDTOP)/include/profile.h $(COM_ERR_DEPS) $(SRCTOP)/include/k5-buf.h \ diff --git a/src/lib/crypto/dk/dk_aead.c b/src/lib/crypto/dk/dk_aead.c index 1aa27687f..f2f98b670 100644 --- a/src/lib/crypto/dk/dk_aead.c +++ b/src/lib/crypto/dk/dk_aead.c @@ -250,7 +250,7 @@ krb5int_dk_decrypt_iov(const struct krb5_aead_provider *aead, for (i = 0; i < num_data; i++) { const krb5_crypto_iov *iov = &data[i]; - if (ENCRYPT_DATA_IOV(iov)) + if (ENCRYPT_IOV(iov)) cipherlen += iov->data.length; } if (cipherlen % blocksize != 0) diff --git a/src/lib/crypto/dk/dk_decrypt.c b/src/lib/crypto/dk/dk_decrypt.c index c38c4d5bf..b23bd4828 100644 --- a/src/lib/crypto/dk/dk_decrypt.c +++ b/src/lib/crypto/dk/dk_decrypt.c @@ -89,6 +89,12 @@ krb5_dk_decrypt_maybe_trunc_hmac(const struct krb5_enc_provider *enc, else if (hmacsize > hashsize) return KRB5KRB_AP_ERR_BAD_INTEGRITY; + /* Verify input and output lengths. */ + if (input->length < blocksize + hmacsize) + return KRB5_BAD_MSIZE; + if (output->length < input->length - blocksize - hmacsize) + return KRB5_BAD_MSIZE; + enclen = input->length - hmacsize; if ((kedata = (unsigned char *) malloc(keylength)) == NULL) diff --git a/src/lib/crypto/enc_provider/aes.c b/src/lib/crypto/enc_provider/aes.c index d821cf461..f14847252 100644 --- a/src/lib/crypto/enc_provider/aes.c +++ b/src/lib/crypto/enc_provider/aes.c @@ -105,9 +105,11 @@ krb5int_aes_encrypt(const krb5_keyblock *key, const krb5_data *ivec, nblocks = (input->length + BLOCK_SIZE - 1) / BLOCK_SIZE; if (nblocks == 1) { - /* XXX Used for DK function. */ + /* Used when deriving keys. */ + if (input->length < BLOCK_SIZE) + return KRB5_BAD_MSIZE; enc(output->data, input->data, &ctx); - } else { + } else if (nblocks > 1) { unsigned int nleft; for (blockno = 0; blockno < nblocks - 2; blockno++) { @@ -160,9 +162,9 @@ krb5int_aes_decrypt(const krb5_keyblock *key, const krb5_data *ivec, if (nblocks == 1) { if (input->length < BLOCK_SIZE) - abort(); + return KRB5_BAD_MSIZE; dec(output->data, input->data, &ctx); - } else { + } else if (nblocks > 1) { for (blockno = 0; blockno < nblocks - 2; blockno++) { dec(tmp2, input->data + blockno * BLOCK_SIZE, &ctx); @@ -208,6 +210,7 @@ krb5int_aes_encrypt_iov(const krb5_keyblock *key, char tmp[BLOCK_SIZE], tmp2[BLOCK_SIZE]; int nblocks = 0, blockno; size_t input_length, i; + struct iov_block_state input_pos, output_pos; if (aes_enc_key(key->contents, key->length, &ctx) != aes_good) abort(); @@ -224,17 +227,19 @@ krb5int_aes_encrypt_iov(const krb5_keyblock *key, input_length += iov->data.length; } - nblocks = (input_length + BLOCK_SIZE - 1) / BLOCK_SIZE; - - assert(nblocks > 1); + IOV_BLOCK_STATE_INIT(&input_pos); + IOV_BLOCK_STATE_INIT(&output_pos); - { + nblocks = (input_length + BLOCK_SIZE - 1) / BLOCK_SIZE; + if (nblocks == 1) { + krb5int_c_iov_get_block((unsigned char *)tmp, BLOCK_SIZE, + data, num_data, &input_pos); + enc(tmp2, tmp, &ctx); + krb5int_c_iov_put_block(data, num_data, (unsigned char *)tmp2, + BLOCK_SIZE, &output_pos); + } else if (nblocks > 1) { char blockN2[BLOCK_SIZE]; /* second last */ char blockN1[BLOCK_SIZE]; /* last block */ - struct iov_block_state input_pos, output_pos; - - IOV_BLOCK_STATE_INIT(&input_pos); - IOV_BLOCK_STATE_INIT(&output_pos); for (blockno = 0; blockno < nblocks - 2; blockno++) { char blockN[BLOCK_SIZE]; @@ -288,6 +293,7 @@ krb5int_aes_decrypt_iov(const krb5_keyblock *key, char tmp[BLOCK_SIZE], tmp2[BLOCK_SIZE], tmp3[BLOCK_SIZE]; int nblocks = 0, blockno, i; size_t input_length; + struct iov_block_state input_pos, output_pos; CHECK_SIZES; @@ -306,17 +312,19 @@ krb5int_aes_decrypt_iov(const krb5_keyblock *key, input_length += iov->data.length; } - nblocks = (input_length + BLOCK_SIZE - 1) / BLOCK_SIZE; - - assert(nblocks > 1); + IOV_BLOCK_STATE_INIT(&input_pos); + IOV_BLOCK_STATE_INIT(&output_pos); - { + nblocks = (input_length + BLOCK_SIZE - 1) / BLOCK_SIZE; + if (nblocks == 1) { + krb5int_c_iov_get_block((unsigned char *)tmp, BLOCK_SIZE, + data, num_data, &input_pos); + dec(tmp2, tmp, &ctx); + krb5int_c_iov_put_block(data, num_data, (unsigned char *)tmp2, + BLOCK_SIZE, &output_pos); + } else if (nblocks > 1) { char blockN2[BLOCK_SIZE]; /* second last */ char blockN1[BLOCK_SIZE]; /* last block */ - struct iov_block_state input_pos, output_pos; - - IOV_BLOCK_STATE_INIT(&input_pos); - IOV_BLOCK_STATE_INIT(&output_pos); for (blockno = 0; blockno < nblocks - 2; blockno++) { char blockN[BLOCK_SIZE]; diff --git a/src/lib/crypto/old/old_decrypt.c b/src/lib/crypto/old/old_decrypt.c index cfbbd7272..89d6a2a8d 100644 --- a/src/lib/crypto/old/old_decrypt.c +++ b/src/lib/crypto/old/old_decrypt.c @@ -45,8 +45,10 @@ krb5_old_decrypt(const struct krb5_enc_provider *enc, blocksize = enc->block_size; hashsize = hash->hashsize; + /* Verify input and output lengths. */ + if (input->length < blocksize + hashsize || input->length % blocksize != 0) + return(KRB5_BAD_MSIZE); plainsize = input->length - blocksize - hashsize; - if (arg_output->length < plainsize) return(KRB5_BAD_MSIZE); diff --git a/src/lib/crypto/raw/raw_decrypt.c b/src/lib/crypto/raw/raw_decrypt.c index 767da1f9f..bfa8e281d 100644 --- a/src/lib/crypto/raw/raw_decrypt.c +++ b/src/lib/crypto/raw/raw_decrypt.c @@ -34,5 +34,7 @@ krb5_raw_decrypt(const struct krb5_enc_provider *enc, const krb5_data *ivec, const krb5_data *input, krb5_data *output) { + if (output->length < input->length) + return KRB5_BAD_MSIZE; return((*(enc->decrypt))(key, ivec, input, output)); } diff --git a/src/lib/crypto/t_short.c b/src/lib/crypto/t_short.c new file mode 100644 index 000000000..9ddfd5e3d --- /dev/null +++ b/src/lib/crypto/t_short.c @@ -0,0 +1,126 @@ +/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */ +/* + * lib/crypto/crypto_tests/t_short.c + * + * Copyright (C) 2009 by the Massachusetts Institute of Technology. + * All rights reserved. + * + * Export of this software from the United States of America may + * require a specific license from the United States Government. + * It is the responsibility of any person or organization contemplating + * export to obtain such a license before exporting. + * + * WITHIN THAT CONSTRAINT, permission to use, copy, modify, and + * distribute this software and its documentation for any purpose and + * without fee is hereby granted, provided that the above copyright + * notice appear in all copies and that both that copyright notice and + * this permission notice appear in supporting documentation, and that + * the name of M.I.T. not be used in advertising or publicity pertaining + * to distribution of the software without specific, written prior + * permission. Furthermore if you modify this software you must label + * your software as modified software and not distribute it in such a + * fashion that it might be confused with the original M.I.T. software. + * M.I.T. makes no representations about the suitability of + * this software for any purpose. It is provided "as is" without express + * or implied warranty. + * + * Tests the outcome of decrypting overly short tokens. This program can be + * run under a tool like valgrind to detect bad memory accesses; when run + * normally by the test suite, it verifies that each operation returns + * KRB5_BAD_MSIZE. + */ + +#include "k5-int.h" + +krb5_enctype interesting_enctypes[] = { + ENCTYPE_DES_CBC_CRC, + ENCTYPE_DES_CBC_MD4, + ENCTYPE_DES_CBC_MD5, + ENCTYPE_DES3_CBC_SHA1, + ENCTYPE_ARCFOUR_HMAC, + ENCTYPE_ARCFOUR_HMAC_EXP, + ENCTYPE_AES256_CTS_HMAC_SHA1_96, + ENCTYPE_AES128_CTS_HMAC_SHA1_96, + 0 +}; + +/* Abort if an operation unexpectedly fails. */ +static void +x(krb5_error_code code) +{ + if (code != 0) + abort(); +} + +/* Abort if a decrypt operation doesn't have the expected result. */ +static void +check_decrypt_result(krb5_error_code code, size_t len, size_t min_len) +{ + if (len < min_len) { + /* Undersized tokens should always result in BAD_MSIZE. */ + if (code != KRB5_BAD_MSIZE) + abort(); + } else { + /* Min-size tokens should succeed or fail the integrity check. */ + if (code != 0 && code != KRB5KRB_AP_ERR_BAD_INTEGRITY) + abort(); + } +} + +static void +test_enctype(krb5_enctype enctype) +{ + krb5_error_code ret; + krb5_keyblock keyblock; + krb5_enc_data input; + krb5_data output; + krb5_crypto_iov iov[2]; + unsigned int dummy; + size_t min_len, len; + + printf("Testing enctype %d\n", (int) enctype); + x(krb5_c_encrypt_length(NULL, enctype, 0, &min_len)); + x(krb5_c_make_random_key(NULL, enctype, &keyblock)); + input.enctype = enctype; + + /* Try each length up to the minimum length. */ + for (len = 0; len <= min_len; len++) { + input.ciphertext.data = calloc(len, 1); + input.ciphertext.length = len; + output.data = calloc(len, 1); + output.length = len; + + /* Attempt a normal decryption. */ + ret = krb5_c_decrypt(NULL, &keyblock, 0, NULL, &input, &output); + check_decrypt_result(ret, len, min_len); + + if (krb5_c_crypto_length(NULL, enctype, KRB5_CRYPTO_TYPE_HEADER, + &dummy) == 0) { + /* Attempt an IOV stream decryption. */ + iov[0].flags = KRB5_CRYPTO_TYPE_STREAM; + iov[0].data = input.ciphertext; + iov[1].flags = KRB5_CRYPTO_TYPE_DATA; + iov[1].data.data = NULL; + iov[1].data.length = 0; + ret = krb5_c_decrypt_iov(NULL, &keyblock, 0, NULL, iov, 2); + check_decrypt_result(ret, len, min_len); + } + + free(input.ciphertext.data); + free(output.data); + } +} + +int +main(int argc, char **argv) +{ + int i; + krb5_data notrandom; + + notrandom.data = "notrandom"; + notrandom.length = 9; + krb5_c_random_seed(NULL, ¬random); + for (i = 0; interesting_enctypes[i]; i++) + test_enctype(interesting_enctypes[i]); + return 0; +} -- 2.26.2