From b6a29fec8e9f283aec0b82ca22328014725d0d7f Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Fri, 21 Jan 2022 10:58:46 -0500 Subject: Avoid passing null for asprintf strings It is undefined behavior to pass null to a printf function for a %.*s substitution, even if the accompanying length is zero. OpenBSD generates syslog warnings from libc when it sees a null pointer in a string substitution (reported by Nathanael Rensen). krb5_sname_to_principal() passes a null pointer in the usual case where there is no port trailer. Address this case and others where we use asprintf() with %.*s substitutions and might pass null, either by avoiding the use of asprintf() or by ensuring that the pointer isn't null. ticket: 9047 (new) --- src/kadmin/server/server_stubs.c | 6 ++++-- src/lib/krb5/krb/get_in_tkt.c | 5 ++--- src/lib/krb5/krb/preauth_otp.c | 41 +++++++++++++++++++--------------------- src/lib/krb5/os/sn2princ.c | 2 +- 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/kadmin/server/server_stubs.c b/src/kadmin/server/server_stubs.c index d5a25e5..ef7e809 100644 --- a/src/kadmin/server/server_stubs.c +++ b/src/kadmin/server/server_stubs.c @@ -4,7 +4,7 @@ * */ -#include +#include #include #include #include /* for gss_nt_krb5_name */ @@ -219,6 +219,7 @@ static gss_name_t acceptor_name(gss_ctx_id_t context) static int gss_to_krb5_name(kadm5_server_handle_t handle, gss_name_t gss_name, krb5_principal *princ) { + krb5_error_code ret; OM_uint32 minor_stat; gss_buffer_desc gss_str; int success; @@ -226,7 +227,8 @@ static int gss_to_krb5_name(kadm5_server_handle_t handle, if (gss_name_to_string(gss_name, &gss_str) != 0) return 0; - if (asprintf(&s, "%.*s", (int)gss_str.length, (char *)gss_str.value) < 0) { + s = k5memdup0(gss_str.value, gss_str.length, &ret); + if (s == NULL) { gss_release_buffer(&minor_stat, &gss_str); return 0; } diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c index 4195a55..8b5ab59 100644 --- a/src/lib/krb5/krb/get_in_tkt.c +++ b/src/lib/krb5/krb/get_in_tkt.c @@ -1183,7 +1183,6 @@ read_cc_config_in_data(krb5_context context, krb5_init_creds_context ctx) krb5_data config; char *encoded; krb5_error_code code; - int i; krb5_ccache in_ccache = k5_gic_opt_get_in_ccache(ctx->opt); k5_json_release(ctx->cc_config_in); @@ -1198,9 +1197,9 @@ read_cc_config_in_data(krb5_context context, krb5_init_creds_context ctx) if (code) return code; - i = asprintf(&encoded, "%.*s", (int)config.length, config.data); + encoded = k5memdup0(config.data, config.length, &code); krb5_free_data_contents(context, &config); - if (i < 0) + if (encoded == NULL) return ENOMEM; code = k5_json_decode(encoded, &val); diff --git a/src/lib/krb5/krb/preauth_otp.c b/src/lib/krb5/krb/preauth_otp.c index 13e5846..5305d9a 100644 --- a/src/lib/krb5/krb/preauth_otp.c +++ b/src/lib/krb5/krb/preauth_otp.c @@ -504,38 +504,33 @@ prompt_for_tokeninfo(krb5_context context, krb5_prompter_fct prompter, void *prompter_data, krb5_otp_tokeninfo **tis, krb5_otp_tokeninfo **out_ti) { - char *banner = NULL, *tmp, response[1024]; + char response[1024]; krb5_otp_tokeninfo *ti = NULL; krb5_error_code retval = 0; + struct k5buf buf; int i = 0, j = 0; + k5_buf_init_dynamic(&buf); + k5_buf_add(&buf, _("Please choose from the following:\n")); for (i = 0; tis[i] != NULL; i++) { - if (asprintf(&tmp, "%s\t%d. %s %.*s\n", - banner ? banner : - _("Please choose from the following:\n"), - i + 1, _("Vendor:"), tis[i]->vendor.length, - tis[i]->vendor.data) < 0) { - free(banner); - return ENOMEM; - } - - free(banner); - banner = tmp; + k5_buf_add_fmt(&buf, "\t%d. %s ", i + 1, _("Vendor:")); + k5_buf_add_len(&buf, tis[i]->vendor.data, tis[i]->vendor.length); + k5_buf_add(&buf, "\n"); } + if (k5_buf_status(&buf) != 0) + return ENOMEM; do { - retval = doprompt(context, prompter, prompter_data, - banner, _("Enter #"), response, sizeof(response)); - if (retval != 0) { - free(banner); - return retval; - } + retval = doprompt(context, prompter, prompter_data, buf.data, + _("Enter #"), response, sizeof(response)); + if (retval != 0) + goto cleanup; errno = 0; j = strtol(response, NULL, 0); if (errno != 0) { - free(banner); - return errno; + retval = errno; + goto cleanup; } if (j < 1 || j > i) continue; @@ -543,9 +538,11 @@ prompt_for_tokeninfo(krb5_context context, krb5_prompter_fct prompter, ti = tis[--j]; } while (ti == NULL); - free(banner); *out_ti = ti; - return 0; + +cleanup: + k5_buf_free(&buf); + return retval; } /* Builds a challenge string from the given tokeninfo. */ diff --git a/src/lib/krb5/os/sn2princ.c b/src/lib/krb5/os/sn2princ.c index 93c1559..3a20e90 100644 --- a/src/lib/krb5/os/sn2princ.c +++ b/src/lib/krb5/os/sn2princ.c @@ -174,7 +174,7 @@ split_trailer(const krb5_data *data, krb5_data *host, krb5_data *trailer) * IPv6 address will have more than one colon, so don't accept that. */ if (p == NULL || tlen == 1 || memchr(p + 1, ':', tlen - 1) != NULL) { *host = *data; - *trailer = empty_data(); + *trailer = make_data("", 0); } else { *host = make_data(data->data, p - data->data); *trailer = make_data(p, tlen); -- cgit v1.1