[29593] in CVS-changelog-for-Kerberos-V5
krb5 commit: Improve checking of decoded DB2 principal values
daemon@ATHENA.MIT.EDU (Greg Hudson)
Mon Aug 29 13:12:01 2016
Date: Mon, 29 Aug 2016 13:11:58 -0400
From: Greg Hudson <ghudson@mit.edu>
Message-Id: <201608291711.u7THBwFZ008091@drugstore.mit.edu>
To: cvs-krb5@mit.edu
Reply-To: krbdev@mit.edu
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: cvs-krb5-bounces@mit.edu
https://github.com/krb5/krb5/commit/e3d9f03a658e247dbb43cb345aa93a28782fd995
commit e3d9f03a658e247dbb43cb345aa93a28782fd995
Author: Greg Hudson <ghudson@mit.edu>
Date: Tue Aug 23 13:41:00 2016 -0400
Improve checking of decoded DB2 principal values
In krb5_decode_princ_entry(), verify the length of the principal name
before calling krb5_parse_name() or strlen(), to avoid a possible
buffer read overrun. Check all length fields for negative values.
Avoid performing arithmetic as part of bounds checks. If the value of
key_data_ver is unexpected, return KRB5_KDB_BAD_VERSION instead of
aborting.
ticket: 8481 (new)
target_version: 1.14-next
target_version: 1.13-next
src/plugins/kdb/db2/kdb_xdr.c | 39 +++++++++++++++++++++++++--------------
1 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/src/plugins/kdb/db2/kdb_xdr.c b/src/plugins/kdb/db2/kdb_xdr.c
index c9f5f04..e476e10 100644
--- a/src/plugins/kdb/db2/kdb_xdr.c
+++ b/src/plugins/kdb/db2/kdb_xdr.c
@@ -250,10 +250,11 @@ krb5_decode_princ_entry(krb5_context context, krb5_data *content,
/* First do the easy stuff */
nextloc = (unsigned char *)content->data;
sizeleft = content->length;
- if ((sizeleft -= KRB5_KDB_V1_BASE_LENGTH) < 0) {
+ if (sizeleft < KRB5_KDB_V1_BASE_LENGTH) {
retval = KRB5_KDB_TRUNCATED_RECORD;
goto error_out;
}
+ sizeleft -= KRB5_KDB_V1_BASE_LENGTH;
/* Base Length */
krb5_kdb_decode_int16(nextloc, entry->len);
@@ -322,32 +323,35 @@ krb5_decode_princ_entry(krb5_context context, krb5_data *content,
* Get the principal name for the entry
* (stored as a string which gets unparsed.)
*/
- if ((sizeleft -= 2) < 0) {
+ if (sizeleft < 2) {
retval = KRB5_KDB_TRUNCATED_RECORD;
goto error_out;
}
+ sizeleft -= 2;
i = 0;
krb5_kdb_decode_int16(nextloc, i16);
i = (int) i16;
nextloc += 2;
-
- if ((retval = krb5_parse_name(context, (char *)nextloc, &(entry->princ))))
- goto error_out;
- if (((size_t) i != (strlen((char *)nextloc) + 1)) || (sizeleft < i)) {
+ if (i <= 0 || i > sizeleft || nextloc[i - 1] != '\0' ||
+ memchr((char *)nextloc, '\0', i - 1) != NULL) {
retval = KRB5_KDB_TRUNCATED_RECORD;
goto error_out;
}
+
+ if ((retval = krb5_parse_name(context, (char *)nextloc, &(entry->princ))))
+ goto error_out;
sizeleft -= i;
nextloc += i;
/* tl_data is a linked list */
tl_data = &entry->tl_data;
for (i = 0; i < entry->n_tl_data; i++) {
- if ((sizeleft -= 4) < 0) {
+ if (sizeleft < 4) {
retval = KRB5_KDB_TRUNCATED_RECORD;
goto error_out;
}
+ sizeleft -= 4;
if ((*tl_data = (krb5_tl_data *)
malloc(sizeof(krb5_tl_data))) == NULL) {
retval = ENOMEM;
@@ -360,10 +364,12 @@ krb5_decode_princ_entry(krb5_context context, krb5_data *content,
krb5_kdb_decode_int16(nextloc, (*tl_data)->tl_data_length);
nextloc += 2;
- if ((sizeleft -= (*tl_data)->tl_data_length) < 0) {
+ if ((*tl_data)->tl_data_length < 0 ||
+ (*tl_data)->tl_data_length > sizeleft) {
retval = KRB5_KDB_TRUNCATED_RECORD;
goto error_out;
}
+ sizeleft -= (*tl_data)->tl_data_length;
(*tl_data)->tl_data_contents =
k5memdup(nextloc, (*tl_data)->tl_data_length, &retval);
if ((*tl_data)->tl_data_contents == NULL)
@@ -382,10 +388,11 @@ krb5_decode_princ_entry(krb5_context context, krb5_data *content,
krb5_key_data * key_data;
int j;
- if ((sizeleft -= 4) < 0) {
+ if (sizeleft < 4) {
retval = KRB5_KDB_TRUNCATED_RECORD;
goto error_out;
}
+ sizeleft -= 4;
key_data = entry->key_data + i;
memset(key_data, 0, sizeof(krb5_key_data));
krb5_kdb_decode_int16(nextloc, key_data->key_data_ver);
@@ -394,21 +401,25 @@ krb5_decode_princ_entry(krb5_context context, krb5_data *content,
nextloc += 2;
/* key_data_ver determins number of elements and how to unparse them. */
- if (key_data->key_data_ver <= KRB5_KDB_V1_KEY_DATA_ARRAY) {
+ if (key_data->key_data_ver >= 0 &&
+ key_data->key_data_ver <= KRB5_KDB_V1_KEY_DATA_ARRAY) {
for (j = 0; j < key_data->key_data_ver; j++) {
- if ((sizeleft -= 4) < 0) {
+ if (sizeleft < 4) {
retval = KRB5_KDB_TRUNCATED_RECORD;
goto error_out;
}
+ sizeleft -= 4;
krb5_kdb_decode_int16(nextloc, key_data->key_data_type[j]);
nextloc += 2;
krb5_kdb_decode_int16(nextloc, key_data->key_data_length[j]);
nextloc += 2;
- if ((sizeleft -= key_data->key_data_length[j]) < 0) {
+ if (key_data->key_data_length[j] < 0 ||
+ key_data->key_data_length[j] > sizeleft) {
retval = KRB5_KDB_TRUNCATED_RECORD;
goto error_out;
}
+ sizeleft -= key_data->key_data_length[j];
if (key_data->key_data_length[j]) {
key_data->key_data_contents[j] =
k5memdup(nextloc, key_data->key_data_length[j],
@@ -419,8 +430,8 @@ krb5_decode_princ_entry(krb5_context context, krb5_data *content,
}
}
} else {
- /* This isn't right. I'll fix it later */
- abort();
+ retval = KRB5_KDB_BAD_VERSION;
+ goto error_out;
}
}
*entry_ptr = entry;
_______________________________________________
cvs-krb5 mailing list
cvs-krb5@mit.edu
https://mailman.mit.edu/mailman/listinfo/cvs-krb5
