From bb75ff893cd5ac8c4e0cc72ab998dd90d56153d6 Mon Sep 17 00:00:00 2001 From: Ken Raeburn Date: Thu, 12 Oct 2006 00:33:12 +0000 Subject: [PATCH] (add_to_transited): Change the current logic to keep all array references in bounds, assuming that what would've been next[-1] would not be '.'. I haven't fully reexamined the logic, but this seems consistent with the actual current behavior, and the existing test cases. Also, factored out code for copying a string from a krb5_data to a char*. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@18688 dc483132-0cff-0310-8789-dd5450dbe970 --- src/kdc/kdc_util.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c index b8ccce30d..7325d4572 100644 --- a/src/kdc/kdc_util.c +++ b/src/kdc/kdc_util.c @@ -548,6 +548,18 @@ subrealm(char *r1, char *r2) * names. */ +static char * +data2string (krb5_data *d) +{ + char *s; + s = malloc(d->length + 1); + if (s) { + memcpy(s, d->data, d->length); + s[d->length] = 0; + } + return s; +} + krb5_error_code add_to_transited(krb5_data *tgt_trans, krb5_data *new_trans, krb5_principal tgs, krb5_principal client, @@ -570,19 +582,15 @@ add_to_transited(krb5_data *tgt_trans, krb5_data *new_trans, int pl, pl1; /* prefix length */ int added; /* TRUE = new realm has been added */ - if (!(realm = (char *) malloc(krb5_princ_realm(kdc_context, tgs)->length+1))) { - return(ENOMEM); - } - memcpy(realm, krb5_princ_realm(kdc_context, tgs)->data, - krb5_princ_realm(kdc_context, tgs)->length); - realm[krb5_princ_realm(kdc_context, tgs)->length] = '\0'; + realm = data2string(krb5_princ_realm(kdc_context, tgs)); + if (realm == NULL) + return(ENOMEM); - if (!(otrans = (char *) malloc(tgt_trans->length+1))) { - free(realm); - return(ENOMEM); + otrans = data2string(tgt_trans); + if (otrans == NULL) { + free(realm); + return(ENOMEM); } - memcpy(otrans, tgt_trans->data, tgt_trans->length); - otrans[tgt_trans->length] = '\0'; /* Keep track of start so we can free */ otrans_ptr = otrans; @@ -699,10 +707,12 @@ add_to_transited(krb5_data *tgt_trans, krb5_data *new_trans, /* subrealm of the next field too, and we will catch */ /* it in a future iteration. */ - assert(nlst >= 0); - assert(nlst < sizeof(next)); - if ((next[nlst] != '.') && (next[0] != '/') && - (pl = subrealm(exp, realm))) { + /* Note that the second test here is an unsigned comparison, + so the first half (or a cast) is also required. */ + assert(nlst < 0 || nlst < sizeof(next)); + if ((nlst < 0 || next[nlst] != '.') && + (next[0] != '/') && + (pl = subrealm(exp, realm))) { added = TRUE; current[sizeof(current) - 1] = '\0'; if (strlen(current) + (pl>0?pl:-pl) + 2 >= MAX_REALM_LN) { -- 2.26.2