diff --git a/kdc/bx509d.c b/kdc/bx509d.c index a2f5fd652..9ae244d77 100644 --- a/kdc/bx509d.c +++ b/kdc/bx509d.c @@ -620,10 +620,20 @@ static krb5_error_code good_bx509(struct bx509_request_desc *r) { krb5_error_code ret; + const char *fn; size_t bodylen; void *body; - ret = rk_undumpdata(strchr(r->pkix_store, ':') + 1, &body, &bodylen); + /* + * This `fn' thing is just to quiet linters that think "hey, strchr() can + * return NULL so...", but here we've build `r->pkix_store' and know it has + * a ':'. + */ + if (r->pkix_store == NULL) + return bad_503(r, EINVAL, "Internal error"); /* Quiet warnings */ + fn = strchr(r->pkix_store, ':'); + fn = fn ? fn + 1 : r->pkix_store; + ret = rk_undumpdata(fn, &body, &bodylen); if (ret) return bad_503(r, ret, "Could not recover issued certificate " "from PKIX store"); @@ -785,6 +795,7 @@ do_CA(struct bx509_request_desc *r, const char *csr) /* Set CSR */ if ((d.data = malloc(strlen(csr2))) == NULL) { krb5_free_principal(r->context, p); + free(csr2); return bad_enomem(r, ENOMEM); } @@ -832,11 +843,9 @@ do_CA(struct bx509_request_desc *r, const char *csr) ret = store_certs(r->context->hx509ctx, r->pkix_store, certs, NULL); hx509_certs_free(&certs); - if (ret) { - (void) unlink(strchr(r->pkix_store, ':') + 1); - return bad_500(r, ret, - "Failed convert issued certificate and chain to PEM"); - } + if (ret) + return bad_500(r, ret, "Failed to convert issued" + " certificate and chain to PEM"); return 0; } @@ -878,7 +887,7 @@ set_req_desc(struct MHD_Connection *connection, r->from = r->frombuf; r->tgt_addresses.len = 0; r->tgt_addresses.val = 0; - r->hcontext = r->context->hcontext; + r->hcontext = r->context ? r->context->hcontext : NULL; r->config = NULL; r->logf = logfac; r->reqtype = url; @@ -894,9 +903,11 @@ set_req_desc(struct MHD_Connection *connection, r->addr = NULL; r->req = NULL; r->req_life = 0; - r->ret = 0; + r->ret = ret; r->kv = heim_dict_create(10); r->attributes = heim_dict_create(1); + if (ret == 0 && (r->kv == NULL || r->attributes == NULL)) + r->ret = ret = ENOMEM; ci = MHD_get_connection_info(connection, MHD_CONNECTION_INFO_CLIENT_ADDRESS); if (ci) { @@ -920,10 +931,6 @@ set_req_desc(struct MHD_Connection *connection, } - if (ret == 0 && r->kv == NULL) { - krb5_log_msg(r->context, logfac, 1, NULL, "Out of memory"); - ret = ENOMEM; - } return ret; } @@ -932,8 +939,17 @@ clean_req_desc(struct bx509_request_desc *r) { if (!r) return; - if (r->pkix_store) - (void) unlink(strchr(r->pkix_store, ':') + 1); + if (r->pkix_store) { + const char *fn = strchr(r->pkix_store, ':'); + + /* + * This `fn' thing is just to quiet linters that think "hey, strchr() can + * return NULL so...", but here we've build `r->pkix_store' and know it has + * a ':'. + */ + fn = fn ? fn + 1 : r->pkix_store; + (void) unlink(fn); + } krb5_free_addresses(r->context, &r->tgt_addresses); hx509_request_free(&r->req); heim_release(r->reason); @@ -1060,8 +1076,10 @@ find_ccache(krb5_context context, const char *princ, char **ccname) */ if ((s = princ_fs_encode(princ)) == NULL || asprintf(ccname, "FILE:%s/%s.cc", cache_dir, s) == -1 || - *ccname == NULL) + *ccname == NULL) { + free(s); return ENOMEM; + } free(s); if ((ret = krb5_cc_resolve(context, *ccname, &cc))) { @@ -1088,7 +1106,6 @@ static krb5_error_code get_ccache(struct bx509_request_desc *r, krb5_ccache *cc, int *won) { krb5_error_code ret = 0; - struct stat st1, st2; char *temp_ccname = NULL; const char *fn = NULL; time_t life; @@ -1118,6 +1135,7 @@ get_ccache(struct bx509_request_desc *r, krb5_ccache *cc, int *won) if (ret == 0) fn = temp_ccname + sizeof("FILE:") - 1; if (ret == 0) do { + struct stat st1, st2; /* * Open and flock the temp ccache file. * @@ -1130,6 +1148,8 @@ get_ccache(struct bx509_request_desc *r, krb5_ccache *cc, int *won) fd = -1; } errno = 0; + memset(&st1, 0, sizeof(st1)); + memset(&st2, 0xff, sizeof(st2)); if (ret == 0 && ((fd = open(fn, O_RDWR | O_CREAT, 0600)) == -1 || flock(fd, LOCK_EX) == -1 || @@ -1201,7 +1221,8 @@ do_pkinit(struct bx509_request_desc *r, enum k5_creds_kind kind) ret = krb5_cc_new_unique(r->context, "FILE", NULL, &temp_cc); } - ret = krb5_parse_name(r->context, cname, &p); + if (ret == 0) + ret = krb5_parse_name(r->context, cname, &p); if (ret == 0) crealm = krb5_principal_get_realm(r->context, p); if (ret == 0) @@ -1702,7 +1723,8 @@ authorize_TGT_REQ(struct bx509_request_desc *r) return 0; ret = krb5_parse_name(r->context, r->cname, &p); - ret = hx509_request_init(r->context->hx509ctx, &r->req); + if (ret == 0) + ret = hx509_request_init(r->context->hx509ctx, &r->req); if (ret) return bad_500(r, ret, "Out of resources"); heim_audit_addkv((heim_svc_req_desc)r, KDC_AUDIT_VIS, @@ -1792,7 +1814,8 @@ get_tgt(struct bx509_request_desc *r) ret = r->ret; /* k5_get_creds() calls bad_req() */ - ret = k5_get_creds(r, K5_CREDS_EPHEMERAL); + if (ret == 0) + ret = k5_get_creds(r, K5_CREDS_EPHEMERAL); if (ret) return ret; @@ -2027,6 +2050,8 @@ main(int argc, char **argv) argc -= optidx; argv += optidx; + if (argc != 0) + usage(1); if ((errno = pthread_key_create(&k5ctx, k5_free_context))) err(1, "Could not create thread-specific storage"); diff --git a/kdc/cjwt_token_validator.c b/kdc/cjwt_token_validator.c index 68fe01594..93742e5dd 100644 --- a/kdc/cjwt_token_validator.c +++ b/kdc/cjwt_token_validator.c @@ -107,13 +107,13 @@ get_issuer_pubkeys(krb5_context context, if (!previous->length && !current->length && !next->length) krb5_set_error_message(context, save_ret, "Could not read jwk issuer public key files"); - if (current->length == next->length && + if (current->length && current->length == next->length && memcmp(current->data, next->data, next->length) == 0) { free(next->data); next->data = 0; next->length = 0; } - if (current->length == previous->length && + if (current->length && current->length == previous->length && memcmp(current->data, previous->data, previous->length) == 0) { free(previous->data); previous->data = 0; @@ -255,6 +255,11 @@ validate(void *ctx, tokstr = NULL; switch (ret) { case 0: + if (jwt == NULL) { + krb5_set_error_message(context, EINVAL, "JWT validation failed"); + free(defrealm); + return EPERM; + } if (jwt->header.alg == alg_none) { krb5_set_error_message(context, EINVAL, "JWT signature algorithm " "not supported"); diff --git a/kdc/simple_csr_authorizer.c b/kdc/simple_csr_authorizer.c index 1ae9efd67..4adfffc83 100644 --- a/kdc/simple_csr_authorizer.c +++ b/kdc/simple_csr_authorizer.c @@ -228,8 +228,10 @@ authorize(void *ctx, if ((san = string_encode(s)) == NULL || asprintf(&p, "%s/%s/%s-%s", d, princ, prefix, san) == -1 || - p == NULL) + p == NULL) { + free(san); goto enomem; + } ret = stat(p, &st) == -1 ? errno : 0; free(san); free(p); @@ -251,10 +253,8 @@ authorize(void *ctx, ret = hx509_request_get_eku(csr, i, &s); if (ret) break; - if (asprintf(&p, "%s/%s/eku-%s", d, princ, s) == -1 || p == NULL) { - free(princ); - free(s); - } + if (asprintf(&p, "%s/%s/eku-%s", d, princ, s) == -1 || p == NULL) + goto enomem; ret = stat(p, &st) == -1 ? errno : 0; free(p); free(s); diff --git a/kdc/test_kdc_ca.c b/kdc/test_kdc_ca.c index 12b873c03..4d80c96a0 100644 --- a/kdc/test_kdc_ca.c +++ b/kdc/test_kdc_ca.c @@ -137,8 +137,9 @@ main(int argc, char **argv) if (ret == 0) hx509_request_authorize_san(req, i); } - if (ret == HX509_NO_ITEM) - ret = 0; + if (ret && ret != HX509_NO_ITEM) + krb5_err(context, 1, ret, + "Failed to mark requested extensions authorized"); } else if ((ret = kdc_authorize_csr(context, app_string, req, p))) { krb5_err(context, 1, ret, "Requested certificate extensions rejected by policy");