krb5 commit: Avoid passing null for asprintf strings

Greg Hudson ghudson at mit.edu
Thu Jan 27 21:10:54 EST 2022


https://github.com/krb5/krb5/commit/b6a29fec8e9f283aec0b82ca22328014725d0d7f
commit b6a29fec8e9f283aec0b82ca22328014725d0d7f
Author: Greg Hudson <ghudson at mit.edu>
Date:   Fri Jan 21 10:58:46 2022 -0500

    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 <k5-platform.h>
+#include <k5-int.h>
 #include <socket-utils.h>
 #include <gssapi/gssapi.h>
 #include <gssapi/gssapi_krb5.h> /* 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);


More information about the cvs-krb5 mailing list