From: Theodore Tso Date: Thu, 13 Jan 1994 23:19:03 +0000 (+0000) Subject: Fixed memory deallocation/cleanup on error returns X-Git-Tag: krb5-1.0-beta3~42 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=4e1eb866967733c044bb477cf7162cfb40d8a477;p=krb5.git Fixed memory deallocation/cleanup on error returns For the credentials structures, established the convention that any of the Kerberos routines that mutate the credentials structures shall free substructure before replacing it. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@3320 dc483132-0cff-0310-8789-dd5450dbe970 --- diff --git a/src/lib/krb5/krb/copy_tick.c b/src/lib/krb5/krb/copy_tick.c index 8871d1b08..d10fa3bb2 100644 --- a/src/lib/krb5/krb/copy_tick.c +++ b/src/lib/krb5/krb/copy_tick.c @@ -67,7 +67,7 @@ krb5_enc_tkt_part **partto; krb5_free_principal(tempto->client); krb5_free_keyblock(tempto->session); krb5_xfree(tempto); - return retval; + return ENOMEM; } memcpy((char *)tempto->transited.tr_contents.data, (char *)partfrom->transited.tr_contents.data, @@ -111,8 +111,10 @@ krb5_ticket **pto; return ENOMEM; *tempto = *from; retval = krb5_copy_principal(from->server, &tempto->server); - if (retval) + if (retval) { + krb5_xfree(tempto); return retval; + } retval = krb5_copy_data(&from->enc_part.ciphertext, &scratch); if (retval) { krb5_free_principal(tempto->server); diff --git a/src/lib/krb5/krb/gc_2tgt.c b/src/lib/krb5/krb/gc_2tgt.c index 2aca318bc..80b5bc9d8 100644 --- a/src/lib/krb5/krb/gc_2tgt.c +++ b/src/lib/krb5/krb/gc_2tgt.c @@ -119,72 +119,73 @@ register krb5_creds * cred; if (retval) return retval; -#undef cleanup -#define cleanup() {\ - memset((char *)dec_rep->enc_part2->session->contents, 0,\ - dec_rep->enc_part2->session->length);\ - krb5_free_kdc_rep(dec_rep); } - if (dec_rep->msg_type != KRB5_TGS_REP) { - cleanup(); - return KRB5KRB_AP_ERR_MSG_TYPE; + retval = KRB5KRB_AP_ERR_MSG_TYPE; + goto errout; } /* now it's decrypted and ready for prime time */ if (!krb5_principal_compare(dec_rep->client, tgt->client)) { - cleanup(); - return KRB5_KDCREP_MODIFIED; + retval = KRB5_KDCREP_MODIFIED; + goto errout; } /* put pieces into cred-> */ - if (retval = krb5_copy_keyblock_contents(dec_rep->enc_part2->session, - &cred->keyblock)) { - cleanup(); - return retval; + if (cred->keyblock.contents) { + memset(&cred->keyblock.contents, 0, cred->keyblock.length); + krb5_xfree(cred->keyblock.contents); } - memset((char *)dec_rep->enc_part2->session->contents, 0, - dec_rep->enc_part2->session->length); - -#undef cleanup -#define cleanup() {\ - memset((char *)cred->keyblock.contents, 0, cred->keyblock.length);\ - krb5_free_kdc_rep(dec_rep); } + if (retval = krb5_copy_keyblock_contents(dec_rep->enc_part2->session, + &cred->keyblock)) + goto errout; /* Should verify that the ticket is what we asked for. */ cred->times = dec_rep->enc_part2->times; cred->ticket_flags = dec_rep->enc_part2->flags; cred->is_skey = TRUE; - if (dec_rep->enc_part2->caddrs) { - if (retval = krb5_copy_addresses(dec_rep->enc_part2->caddrs, - &cred->addresses)) { - cleanup(); - return retval; - } - } else { + if (cred->addresses) + krb5_free_addresses(cred->addresses); + if (dec_rep->enc_part2->caddrs) + retval = krb5_copy_addresses(dec_rep->enc_part2->caddrs, + &cred->addresses); + else /* no addresses in the list means we got what we had */ - if (retval = krb5_copy_addresses(tgt->addresses, - &cred->addresses)) { - cleanup(); - return retval; - } - } + retval = krb5_copy_addresses(tgt->addresses, &cred->addresses); + if (retval) + goto errout; + + if (cred->server) + krb5_free_principal(cred->server); if (retval = krb5_copy_principal(dec_rep->enc_part2->server, - &cred->server)) { - cleanup(); - return retval; - } + &cred->server)) + goto errout; - if (retval = encode_krb5_ticket(dec_rep->ticket, &scratch)) { - cleanup(); - krb5_free_addresses(cred->addresses); - return retval; - } + if (retval = encode_krb5_ticket(dec_rep->ticket, &scratch)) + goto errout; cred->ticket = *scratch; krb5_xfree(scratch); +errout: + if (retval) { + if (cred->keyblock.contents) { + memset((char *)cred->keyblock.contents, 0, cred->keyblock.length); + krb5_xfree(cred->keyblock.contents); + cred->keyblock.contents = 0; + } + if (cred->addresses) { + krb5_free_addresses(cred->addresses); + cred->addresses = 0; + } + if (cred->server) { + krb5_free_principal(cred->server); + cred->server = 0; + } + } + memset((char *)dec_rep->enc_part2->session->contents, 0, + dec_rep->enc_part2->session->length); krb5_free_kdc_rep(dec_rep); - return 0; + return retval; } /* diff --git a/src/lib/krb5/krb/gc_frm_kdc.c b/src/lib/krb5/krb/gc_frm_kdc.c index 0110328c3..c825efa2f 100644 --- a/src/lib/krb5/krb/gc_frm_kdc.c +++ b/src/lib/krb5/krb/gc_frm_kdc.c @@ -73,7 +73,7 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) { krb5_creds tgt, tgtq; krb5_creds **ret_tgts = 0; - krb5_principal *tgs_list, *next_server; + krb5_principal *tgs_list = 0, *next_server; krb5_principal final_server; krb5_error_code retval; int nservers; @@ -82,6 +82,9 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) /* in case we never get a TGT, zero the return */ *tgts = 0; + + memset((char *)&tgtq, 0, sizeof(tgtq)); + memset((char *)&tgt, 0, sizeof(tgt)); /* * we know that the desired credentials aren't in the cache yet. @@ -99,20 +102,14 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) * realm's KDC; so we use KRB5_TC_MATCH_SRV_NAMEONLY below) */ - /* - * we're sharing some substructure here, which is dangerous. - * Be sure that if you muck with things here that tgtq.* doesn't share - * any substructure before you deallocate/clean up/whatever. - */ - memset((char *)&tgtq, 0, sizeof(tgtq)); - tgtq.client = cred->client; - + if (retval = krb5_copy_principal(cred->client, &tgtq.client)) + goto errout; if (retval = krb5_tgtname(krb5_princ_realm(cred->server), krb5_princ_realm(cred->client), &final_server)) - return retval; - tgtq.server = final_server; + goto errout; + if (retval = krb5_copy_principal(final_server, &tgtq.server)) + goto errout; - /* try to fetch it directly */ retval = krb5_cc_retrieve_cred (ccache, KRB5_TC_MATCH_SRV_NAMEONLY, &tgtq, @@ -120,16 +117,16 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) if (retval != 0) { if (retval != KRB5_CC_NOTFOUND) - goto out; + goto errout; /* don't have the right TGT in the cred cache. Time to iterate across realms to get the right TGT. */ /* get a list of realms to consult */ - retval = krb5_walk_realm_tree(krb5_princ_realm(cred->client), - krb5_princ_realm(cred->server), - &tgs_list, KRB5_REALM_BRANCH_CHAR); - if (retval) - goto out; + if (retval = krb5_walk_realm_tree(krb5_princ_realm(cred->client), + krb5_princ_realm(cred->server), + &tgs_list, KRB5_REALM_BRANCH_CHAR)) + goto errout; + /* walk the list BACKWARDS until we find a cached TGT, then move forward obtaining TGTs until we get the last TGT needed */ @@ -140,16 +137,16 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) /* next_server now points to the last TGT */ for (; next_server >= tgs_list; next_server--) { - tgtq.server = *next_server; + krb5_free_principal(tgtq.server); + if (retval = krb5_copy_principal(*next_server, &tgt.server)) + goto errout; retval = krb5_cc_retrieve_cred (ccache, KRB5_TC_MATCH_SRV_NAMEONLY, &tgtq, &tgt); if (retval) { - if (retval != KRB5_CC_NOTFOUND) { - krb5_free_realm_tree(tgs_list); - goto out; - } + if (retval != KRB5_CC_NOTFOUND) + goto errout; continue; } next_server++; @@ -158,38 +155,38 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) if (next_server < tgs_list) { /* didn't find any */ retval = KRB5_NO_TKT_IN_RLM; - krb5_free_realm_tree(tgs_list); - goto out; + goto errout; } /* allocate storage for TGT pointers. */ ret_tgts = (krb5_creds **)calloc(nservers+1, sizeof(krb5_creds)); if (!ret_tgts) { retval = ENOMEM; - krb5_free_realm_tree(tgs_list); - goto out; + goto errout; } *tgts = ret_tgts; for (nservers = 0; *next_server; next_server++, nservers++) { - krb5_data *tmpdata; if (!valid_keytype(tgt.keyblock.keytype)) { retval = KRB5_PROG_KEYTYPE_NOSUPP; - krb5_free_realm_tree(tgs_list); - goto out; + goto errout; } /* now get the TGTs */ + krb5_free_cred_contents(&tgtq); memset((char *)&tgtq, 0, sizeof(tgtq)); tgtq.times = tgt.times; - tgtq.client = tgt.client; + if (retval = krb5_copy_principal(tgt.client, &tgtq.client)) + goto errout; /* ask each realm for a tgt to the end */ - if (retval = krb5_copy_data(krb5_princ_realm(*next_server), &tmpdata)) { - krb5_free_realm_tree(tgs_list); - goto out; + if (retval = krb5_copy_data(krb5_princ_realm(*next_server), + &tmpdata)) { + goto errout; } + free(krb5_princ_realm(final_server)->data); krb5_princ_set_realm(final_server, tmpdata); - tgtq.server = final_server; + if (retval = krb5_copy_principal(final_server, &tgtq.server)) + goto errout; tgtq.is_skey = FALSE; tgtq.ticket_flags = tgt.ticket_flags; @@ -199,10 +196,9 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) flags2options(tgtq.ticket_flags), etype, krb5_kdc_req_sumtype, - &tgtq)) { - krb5_free_realm_tree(tgs_list); - goto out; - } + &tgtq)) + goto errout; + /* make sure the returned ticket is somewhere in the remaining list, but we can tolerate different expected issuing realms */ while (*++next_server && @@ -210,28 +206,21 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) &(tgtq.server[1]))); if (!next_server) { /* what we got back wasn't in the list! */ - krb5_free_realm_tree(tgs_list); retval = KRB5_KDCREP_MODIFIED; - goto out; + goto errout; } /* save tgt in return array */ - if (retval = krb5_copy_creds(&tgtq, &ret_tgts[nservers])) { - krb5_free_realm_tree(tgs_list); - goto out; - } + if (retval = krb5_copy_creds(&tgtq, &ret_tgts[nservers])) + goto errout; tgt = *ret_tgts[nservers]; returning_tgt = 1; /* don't free it below... */ - tgtq.client = 0; - tgtq.server = 0; - krb5_free_cred_contents(&tgtq); } - krb5_free_realm_tree(tgs_list); } /* got/finally have tgt! */ if (!valid_keytype(tgt.keyblock.keytype)) { retval = KRB5_PROG_KEYTYPE_NOSUPP; - goto out; + goto errout; } etype = krb5_keytype_array[tgt.keyblock.keytype]->system->proto_enctype; @@ -246,10 +235,13 @@ krb5_get_cred_from_kdc (ccache, cred, tgts) etype, krb5_kdc_req_sumtype, cred); - +errout: if (!returning_tgt) krb5_free_cred_contents(&tgt); -out: - krb5_free_principal(final_server); + if (final_server) + krb5_free_principal(final_server); + krb5_free_cred_contents(&tgtq); + if (tgs_list) + krb5_free_realm_tree(tgs_list); return retval; } diff --git a/src/lib/krb5/krb/gc_via_tgt.c b/src/lib/krb5/krb/gc_via_tgt.c index d6ef9119b..23dc668f3 100644 --- a/src/lib/krb5/krb/gc_via_tgt.c +++ b/src/lib/krb5/krb/gc_via_tgt.c @@ -143,6 +143,10 @@ OLDDECLARG(krb5_creds *, cred) return KRB5_KDCREP_MODIFIED; } /* put pieces into cred-> */ + if (cred->keyblock.contents) { + memset(&cred->keyblock.contents, 0, cred->keyblock.length); + krb5_xfree(cred->keyblock.contents); + } if (retval = krb5_copy_keyblock_contents(dec_rep->enc_part2->session, &cred->keyblock)) { cleanup(); @@ -188,6 +192,9 @@ OLDDECLARG(krb5_creds *, cred) cred->ticket_flags = dec_rep->enc_part2->flags; cred->is_skey = FALSE; + if (cred->addresses) { + krb5_free_addresses(cred->addresses); + } if (dec_rep->enc_part2->caddrs) { if (retval = krb5_copy_addresses(dec_rep->enc_part2->caddrs, &cred->addresses)) { @@ -205,7 +212,8 @@ OLDDECLARG(krb5_creds *, cred) /* * Free cred->server before overwriting it. */ - krb5_free_principal(cred->server); + if (cred->server) + krb5_free_principal(cred->server); if (retval = krb5_copy_principal(dec_rep->enc_part2->server, &cred->server)) { cleanup(); diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c index 320a3b208..28ff4739e 100644 --- a/src/lib/krb5/krb/get_in_tkt.c +++ b/src/lib/krb5/krb/get_in_tkt.c @@ -320,6 +320,7 @@ OLDDECLARG(krb5_kdc_rep **, ret_as_reply) if (retval = krb5_copy_addresses(as_reply->enc_part2->caddrs, &creds->addresses)) { cleanup_key(); + krb5_free_kdc_rep(as_reply); return retval; } creds->second_ticket.length = 0; @@ -330,6 +331,7 @@ OLDDECLARG(krb5_kdc_rep **, ret_as_reply) krb5_free_kdc_rep(as_reply); krb5_free_addresses(creds->addresses); cleanup_key(); + krb5_free_kdc_rep(as_reply); return retval; } creds->ticket = *packet; @@ -342,6 +344,7 @@ OLDDECLARG(krb5_kdc_rep **, ret_as_reply) krb5_xfree(creds->ticket.data); krb5_free_addresses(creds->addresses); cleanup_key(); + krb5_free_kdc_rep(as_reply); return retval; } if (ret_as_reply) diff --git a/src/lib/krb5/krb/mk_req.c b/src/lib/krb5/krb/mk_req.c index d9afd1cbf..252ec976d 100644 --- a/src/lib/krb5/krb/mk_req.c +++ b/src/lib/krb5/krb/mk_req.c @@ -69,9 +69,10 @@ krb5_data *outbuf; /* obtain ticket & session key */ memset((char *)&creds, 0, sizeof(creds)); - creds.server = (krb5_principal) server; + if (retval = krb5_copy_principal(server, &creds.server)) + goto errout; if (retval = krb5_cc_get_principal(ccache, &creds.client)) - return(retval); + goto errout; /* creds.times.endtime = 0; -- memset 0 takes care of this zero means "as long as possible" */ /* creds.keyblock.keytype = 0; -- as well as this. @@ -81,7 +82,7 @@ krb5_data *outbuf; if (retval = krb5_get_credentials(krb5_kdc_default_options, ccache, &creds)) - return(retval); + goto errout; retval = krb5_mk_req_extended(ap_req_options, checksum, @@ -92,9 +93,8 @@ krb5_data *outbuf; &creds, 0, /* We don't need the authenticator */ outbuf); - /* creds.server and the rest of the creds.* fields are filled in - by the ccache fetch or the kdc fetch, so we should allow it to be - freed */ + +errout: krb5_free_cred_contents(&creds); return retval; } diff --git a/src/lib/krb5/krb/rd_safe.c b/src/lib/krb5/krb/rd_safe.c index 8e6cd992d..b884e1660 100644 --- a/src/lib/krb5/krb/rd_safe.c +++ b/src/lib/krb5/krb/rd_safe.c @@ -78,11 +78,15 @@ krb5_data *outbuf; #define cleanup() krb5_free_safe(message) - if (!valid_cksumtype(message->checksum->checksum_type)) + if (!valid_cksumtype(message->checksum->checksum_type)) { + cleanup(); return KRB5_PROG_SUMTYPE_NOSUPP; + } if (!is_coll_proof_cksum(message->checksum->checksum_type) || - !is_keyed_cksum(message->checksum->checksum_type)) + !is_keyed_cksum(message->checksum->checksum_type)) { + cleanup(); return KRB5KRB_AP_ERR_INAPP_CKSUM; + } if (!(safe_flags & KRB5_SAFE_NOTIME)) { krb5_donot_replay replay;