From 70296e1f530313283f9a48dd0ec467e5c280a79d Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Thu, 23 Oct 2008 19:59:05 +0000 Subject: [PATCH] Use snprintf instead of strcpy/strcat in many places ticket: 6200 status: open git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@20912 dc483132-0cff-0310-8789-dd5450dbe970 --- src/appl/gssftp/ftp/cmds.c | 6 ++-- src/appl/gssftp/ftpd/ftpd.c | 8 +++-- src/appl/telnet/libtelnet/kerberos5.c | 48 ++++++++++++--------------- src/appl/telnet/libtelnet/spx.c | 16 ++++----- src/appl/telnet/telnetd/sys_term.c | 4 +-- src/clients/ksu/ccache.c | 7 ++-- src/include/k5-platform.h | 16 +++++++++ src/kadmin/ktutil/ktutil_funcs.c | 6 ++-- src/lib/des425/read_passwd.c | 5 ++- src/lib/kdb/kdb_default.c | 13 +++----- src/lib/krb5/ccache/cc_file.c | 3 +- src/lib/krb5/keytab/kt_file.c | 17 +++------- src/lib/krb5/keytab/kt_memory.c | 17 +++------- src/lib/krb5/keytab/kt_srvtab.c | 17 +++------- src/lib/krb5/krb/gic_pwd.c | 5 +-- 15 files changed, 80 insertions(+), 108 deletions(-) diff --git a/src/appl/gssftp/ftp/cmds.c b/src/appl/gssftp/ftp/cmds.c index e73378157..2f7c8310a 100644 --- a/src/appl/gssftp/ftp/cmds.c +++ b/src/appl/gssftp/ftp/cmds.c @@ -66,6 +66,8 @@ static char sccsid[] = "@(#)cmds.c 5.26 (Berkeley) 3/5/91"; #include #include +#include + #ifdef HAVE_GETCWD #define getwd(x) getcwd(x,MAXPATHLEN) #endif @@ -1615,9 +1617,7 @@ void shell(argc, argv) namep = strrchr(shellprog,'/'); if (namep == NULL) namep = shellprog; - (void) strcpy(shellnam,"-"); - (void) strncat(shellnam, ++namep, sizeof(shellnam) - 1 - strlen(shellnam)); - shellnam[sizeof(shellnam) - 1] = '\0'; + (void) snprintf(shellnam, sizeof(shellnam), "-%s", ++namep); if (strcmp(namep, "sh") != 0) shellnam[0] = '+'; if (debug) { diff --git a/src/appl/gssftp/ftpd/ftpd.c b/src/appl/gssftp/ftpd/ftpd.c index 485332d3b..4405e9b17 100644 --- a/src/appl/gssftp/ftpd/ftpd.c +++ b/src/appl/gssftp/ftpd/ftpd.c @@ -102,6 +102,8 @@ static char sccsid[] = "@(#)ftpd.c 5.40 (Berkeley) 7/2/91"; #include "pathnames.h" #include +#include + #ifdef NEED_SETENV extern int setenv(char *, char *, int); #endif @@ -1752,14 +1754,14 @@ statcmd() sin4 = &pasv_addr; goto printaddr; } else if (usedefault == 0) { - strcpy(str, " PORT"); sin4 = &data_dest; printaddr: a = (u_char *) &sin4->sin_addr; p = (u_char *) &sin4->sin_port; #define UC(b) (((int) b) & 0xff) - sprintf(&str[strlen(str)], " (%d,%d,%d,%d,%d,%d)", UC(a[0]), - UC(a[1]), UC(a[2]), UC(a[3]), UC(p[0]), UC(p[1])); + snprintf(str, sizeof(str), " PORT (%d,%d,%d,%d,%d,%d)", + UC(a[0]), UC(a[1]), UC(a[2]), UC(a[3]), UC(p[0]), + UC(p[1])); #undef UC } else strcpy(str, " No data connection"); diff --git a/src/appl/telnet/libtelnet/kerberos5.c b/src/appl/telnet/libtelnet/kerberos5.c index ff94d0112..aec975670 100644 --- a/src/appl/telnet/libtelnet/kerberos5.c +++ b/src/appl/telnet/libtelnet/kerberos5.c @@ -66,6 +66,7 @@ #include #include #include "krb5.h" +#include "k5-platform.h" #include "com_err.h" #include @@ -439,9 +440,9 @@ kerberos5_is(ap, data, cnt) r = krb5_rd_req(telnet_context, &auth_context, &auth, NULL, keytabid, NULL, &ticket); if (r) { - (void) strcpy(errbuf, "krb5_rd_req failed: "); - errbuf[sizeof(errbuf) - 1] = '\0'; - (void) strncat(errbuf, error_message(r), sizeof(errbuf) - 1 - strlen(errbuf)); + (void) snprintf(errbuf, sizeof(errbuf), + "krb5_rd_req failed: %s", + error_message(r)); goto errout; } @@ -479,11 +480,10 @@ kerberos5_is(ap, data, cnt) auth_context, &authenticator); if (r) { - (void) strcpy(errbuf, - "krb5_auth_con_getauthenticator failed: "); - errbuf[sizeof(errbuf) - 1] = '\0'; - (void) strncat(errbuf, error_message(r), sizeof(errbuf) - 1 - strlen(errbuf)); - goto errout; + (void) snprintf(errbuf, sizeof(errbuf), + "krb5_auth_con_getauthenticator failed: %s", + error_message(r)); + goto errout; } if ((ap->way & AUTH_ENCRYPT_MASK) == AUTH_ENCRYPT_ON && !authenticator->checksum) { @@ -502,9 +502,9 @@ kerberos5_is(ap, data, cnt) r = krb5_auth_con_getkey(telnet_context, auth_context, &key); if (r) { - (void) strcpy(errbuf, "krb5_auth_con_getkey failed: "); - errbuf[sizeof(errbuf) - 1] = '\0'; - (void) strncat(errbuf, error_message(r), sizeof(errbuf) - 1 - strlen(errbuf)); + (void) snprintf(errbuf, sizeof(errbuf), + "krb5_auth_con_getkey failed: %s", + error_message(r)); goto errout; } r = krb5_verify_checksum(telnet_context, @@ -521,10 +521,9 @@ kerberos5_is(ap, data, cnt) * present at this time. */ if (r) { - (void) strcpy(errbuf, - "checksum verification failed: "); - errbuf[sizeof(errbuf) - 1] = '\0'; - (void) strncat(errbuf, error_message(r), sizeof(errbuf) - 1 - strlen(errbuf)); + (void) snprintf(errbuf, sizeof(errbuf), + "checksum verification failed: %s", + error_message(r)); goto errout; } krb5_free_keyblock(telnet_context, key); @@ -534,9 +533,9 @@ kerberos5_is(ap, data, cnt) /* do ap_rep stuff here */ if ((r = krb5_mk_rep(telnet_context, auth_context, &outbuf))) { - (void) strcpy(errbuf, "Make reply failed: "); - errbuf[sizeof(errbuf) - 1] = '\0'; - (void) strncat(errbuf, error_message(r), sizeof(errbuf) - 1 - strlen(errbuf)); + (void) snprintf(errbuf, sizeof(errbuf), + "Make reply failed: %s", + error_message(r)); goto errout; } @@ -588,11 +587,10 @@ kerberos5_is(ap, data, cnt) &inbuf, ticket))) { char kerrbuf[128]; - - (void) strcpy(kerrbuf, "Read forwarded creds failed: "); - kerrbuf[sizeof(kerrbuf) - 1] = '\0'; - (void) strncat(kerrbuf, error_message(r), - sizeof(kerrbuf) - 1 - strlen(kerrbuf)); + + (void) snprintf(kerrbuf, sizeof(kerrbuf), + "Read forwarded creds failed: %s", + error_message(r)); Data(ap, KRB_FORWARD_REJECT, kerrbuf, -1); if (auth_debug_mode) printf( @@ -617,9 +615,7 @@ kerberos5_is(ap, data, cnt) { char eerrbuf[329]; - strcpy(eerrbuf, "telnetd: "); - eerrbuf[sizeof(eerrbuf) - 1] = '\0'; - strncat(eerrbuf, errbuf, sizeof(eerrbuf) - 1 - strlen(eerrbuf)); + snprintf(eerrbuf, sizeof(eerrbuf), "telnetd: %s", errbuf); Data(ap, KRB_REJECT, eerrbuf, -1); } if (auth_debug_mode) diff --git a/src/appl/telnet/libtelnet/spx.c b/src/appl/telnet/libtelnet/spx.c index b3e0e9dfc..b12bd09cf 100644 --- a/src/appl/telnet/libtelnet/spx.c +++ b/src/appl/telnet/libtelnet/spx.c @@ -71,6 +71,7 @@ #include #include #include "gssapi_defs.h" +#include "k5-platform.h" #ifdef __STDC__ #include #endif @@ -172,9 +173,8 @@ spx_init(ap, server) if (server) { str_data[3] = TELQUAL_REPLY; gethostname(lhostname, sizeof(lhostname)); - strcpy(targ_printable, "SERVICE:rcmd@"); - strncat(targ_printable, lhostname, sizeof(targ_printable) - 1 - 13); - targ_printable[sizeof(targ_printable) - 1] = '\0'; + snprintf(targ_printable, sizeof(targ_printable), + "SERVICE:rcmd@%s", lhostname); input_name_buffer.length = strlen(targ_printable); input_name_buffer.value = targ_printable; major_status = gss_import_name(&status, @@ -216,9 +216,8 @@ spx_send(ap) char *address; printf("[ Trying SPX ... ]\n"); - strcpy(targ_printable, "SERVICE:rcmd@"); - strncat(targ_printable, RemoteHostName, sizeof(targ_printable) - 1 - 13); - targ_printable[sizeof(targ_printable) - 1] = '\0'; + snprintf(targ_printable, sizeof(targ_printable), "SERVICE:rcmd@%s", + RemoteHostName); input_name_buffer.length = strlen(targ_printable); input_name_buffer.value = targ_printable; @@ -325,9 +324,8 @@ spx_is(ap, data, cnt) gethostname(lhostname, sizeof(lhostname)); - strcpy(targ_printable, "SERVICE:rcmd@"); - strncat(targ_printable, lhostname, sizeof(targ_printable) - 1 - 13); - targ_printable[sizeof(targ_printable) - 1] = '\0'; + snprintf(targ_printable, sizeof(targ_printable), + "SERVICE:rcmd@%s", lhostname); input_name_buffer.length = strlen(targ_printable); input_name_buffer.value = targ_printable; diff --git a/src/appl/telnet/telnetd/sys_term.c b/src/appl/telnet/telnetd/sys_term.c index d78c2e83d..d86bafd8f 100644 --- a/src/appl/telnet/telnetd/sys_term.c +++ b/src/appl/telnet/telnetd/sys_term.c @@ -1255,9 +1255,7 @@ start_login(host, autologin, name) if (term == NULL || term[0] == 0) { term = "-"; } else { - strcpy(termbuf, "TERM="); - strncat(termbuf, term, sizeof(termbuf) - 6); - termbuf[sizeof(termbuf) - 1] = '\0'; + snprintf(termbuf, sizeof(termbuf), "TERM=%s", term); term = termbuf; } argv = addarg(argv, term); diff --git a/src/clients/ksu/ccache.c b/src/clients/ksu/ccache.c index ec8f2af0e..8fd11a3c8 100644 --- a/src/clients/ksu/ccache.c +++ b/src/clients/ksu/ccache.c @@ -373,7 +373,7 @@ krb5_get_login_princ(luser, princ_list) FILE *fp; char * linebuf; char *newline; - int gobble; + int gobble, result; char ** buf_out; struct stat st_temp; int count = 0, chunk_count = 1; @@ -383,12 +383,11 @@ krb5_get_login_princ(luser, princ_list) if ((pwd = getpwnam(luser)) == NULL) { return 0; } - if (strlen(pwd->pw_dir) + sizeof("/.k5login") > MAXPATHLEN) { + result = snprintf(pbuf, sizeof(pbuf), "%s/.k5login", pwd->pw_dir); + if (SNPRINTF_OVERFLOW(result, sizeof(pbuf))) { fprintf (stderr, "home directory path for %s too long\n", luser); exit (1); } - (void) strcpy(pbuf, pwd->pw_dir); - (void) strcat(pbuf, "/.k5login"); if (stat(pbuf, &st_temp)) { /* not accessible */ return 0; diff --git a/src/include/k5-platform.h b/src/include/k5-platform.h index 4a2b1aef7..279d6fd96 100644 --- a/src/include/k5-platform.h +++ b/src/include/k5-platform.h @@ -920,6 +920,22 @@ extern int asprintf(char **, const char *, ...) #endif /* have vasprintf and prototype? */ +/* Return true if the snprintf return value RESULT reflects a buffer + overflow for the buffer size SIZE. + + We cast the result to unsigned int for two reasons. First, old + implementations of snprintf (such as the one in Solaris 9 and + prior) return -1 on a buffer overflow. Casting the result to -1 + will convert that value to UINT_MAX, which should compare larger + than any reasonable buffer size. Second, comparing signed and + unsigned integers will generate warnings with some compilers, and + can have unpredictable results, particularly when the relative + widths of the types is not known (size_t may be the same width as + int or larger). +*/ +#define SNPRINTF_OVERFLOW(result, size) \ + ((unsigned int)(result) >= (size_t)(size)) + #ifndef HAVE_MKSTEMP extern int krb5int_mkstemp(char *); #define mkstemp krb5int_mkstemp diff --git a/src/kadmin/ktutil/ktutil_funcs.c b/src/kadmin/ktutil/ktutil_funcs.c index 649002e21..a9106debc 100644 --- a/src/kadmin/ktutil/ktutil_funcs.c +++ b/src/kadmin/ktutil/ktutil_funcs.c @@ -317,11 +317,11 @@ krb5_error_code ktutil_write_keytab(context, list, name) krb5_keytab kt; char ktname[MAXPATHLEN+sizeof("WRFILE:")+1]; krb5_error_code retval = 0; + int result; - strcpy(ktname, "WRFILE:"); - if (strlen (name) >= MAXPATHLEN) + result = snprintf(ktname, sizeof(ktname), "WRFILE:%s", name); + if (SNPRINTF_OVERFLOW(result, sizeof(ktname))) return ENAMETOOLONG; - strncat (ktname, name, MAXPATHLEN); retval = krb5_kt_resolve(context, ktname, &kt); if (retval) return retval; diff --git a/src/lib/des425/read_passwd.c b/src/lib/des425/read_passwd.c index e1b4c713c..bdcb32999 100644 --- a/src/lib/des425/read_passwd.c +++ b/src/lib/des425/read_passwd.c @@ -114,9 +114,8 @@ des_read_pw_string(s, max, prompt, verify) char prompt2[BUFSIZ]; if (verify) { - strcpy(prompt2, "Verifying, please re-enter "); - strncat(prompt2, prompt, sizeof(prompt2)-(strlen(prompt2)+1)); - prompt2[sizeof(prompt2)-1] = '\0'; + snprintf(prompt2, sizeof(prompt2), "Verifying, please re-enter %s", + prompt); } ok = des_rd_pwstr_2prompt(s, max, prompt, verify ? prompt2 : 0); return ok; diff --git a/src/lib/kdb/kdb_default.c b/src/lib/kdb/kdb_default.c index d6f724ce0..f7a855b72 100644 --- a/src/lib/kdb/kdb_default.c +++ b/src/lib/kdb/kdb_default.c @@ -152,11 +152,8 @@ krb5_def_store_mkey(krb5_context context, int statrc; if (!keyfile) { - (void) strcpy(defkeyfile, DEFAULT_KEYFILE_STUB); - (void) strncat(defkeyfile, realm->data, - min(sizeof(defkeyfile)-sizeof(DEFAULT_KEYFILE_STUB)-1, - realm->length)); - defkeyfile[sizeof(defkeyfile) - 1] = '\0'; + (void) snprintf(defkeyfile, sizeof(defkeyfile), "%s%s", + DEFAULT_KEYFILE_STUB, realm->data); keyfile = defkeyfile; } @@ -392,10 +389,8 @@ krb5_db_def_fetch_mkey(krb5_context context, if (db_args != NULL) { (void) strncpy(keyfile, db_args, sizeof(keyfile)); } else { - (void) strcpy(keyfile, DEFAULT_KEYFILE_STUB); - (void) strncat(keyfile, realm->data, - min(sizeof(keyfile)-sizeof(DEFAULT_KEYFILE_STUB)-1, - realm->length)); + (void) snprintf(keyfile, sizeof(keyfile), "%s%s", + DEFAULT_KEYFILE_STUB, realm->data); } /* null terminate no matter what */ keyfile[sizeof(keyfile) - 1] = '\0'; diff --git a/src/lib/krb5/ccache/cc_file.c b/src/lib/krb5/ccache/cc_file.c index 22c01b8b7..b5a099059 100644 --- a/src/lib/krb5/ccache/cc_file.c +++ b/src/lib/krb5/ccache/cc_file.c @@ -1997,8 +1997,7 @@ krb5_fcc_generate_new (krb5_context context, krb5_ccache *id) if (kret) return kret; - (void) strcpy(scratch, TKT_ROOT); - (void) strcat(scratch, "XXXXXX"); + (void) snprintf(scratch, sizeof(scratch), "%sXXXXXX", TKT_ROOT); ret = mkstemp(scratch); if (ret == -1) { k5_cc_mutex_unlock(context, &krb5int_cc_file_mutex); diff --git a/src/lib/krb5/keytab/kt_file.c b/src/lib/krb5/keytab/kt_file.c index bf394d887..83fb26485 100644 --- a/src/lib/krb5/keytab/kt_file.c +++ b/src/lib/krb5/keytab/kt_file.c @@ -440,21 +440,12 @@ krb5_ktfile_get_name(krb5_context context, krb5_keytab id, char *name, unsigned * trt will happen if the name is passed back to resolve. */ { - memset(name, 0, len); - - if (len < strlen(id->ops->prefix)+2) - return(KRB5_KT_NAME_TOOLONG); - strcpy(name, id->ops->prefix); - name += strlen(id->ops->prefix); - name[0] = ':'; - name++; - len -= strlen(id->ops->prefix)+1; + int result; - if (len < strlen(KTFILENAME(id))+1) + memset(name, 0, len); + result = snprintf(name, len, "%s:%s", id->ops->prefix, KTFILENAME(id)); + if (SNPRINTF_OVERFLOW(result, len)) return(KRB5_KT_NAME_TOOLONG); - strcpy(name, KTFILENAME(id)); - /* strcpy will NUL-terminate the destination */ - return(0); } diff --git a/src/lib/krb5/keytab/kt_memory.c b/src/lib/krb5/keytab/kt_memory.c index 1b4af9bd6..53d15edd8 100644 --- a/src/lib/krb5/keytab/kt_memory.c +++ b/src/lib/krb5/keytab/kt_memory.c @@ -472,21 +472,12 @@ krb5_mkt_get_entry(krb5_context context, krb5_keytab id, krb5_error_code KRB5_CALLCONV krb5_mkt_get_name(krb5_context context, krb5_keytab id, char *name, unsigned int len) { - memset(name, 0, len); - - if (len < strlen(id->ops->prefix)+2) - return(KRB5_KT_NAME_TOOLONG); - strcpy(name, id->ops->prefix); - name += strlen(id->ops->prefix); - name[0] = ':'; - name++; - len -= strlen(id->ops->prefix)+1; + int result; - if (len < strlen(KTNAME(id))+1) + memset(name, 0, len); + result = snprintf(name, len, "%s:%s", id->ops->prefix, KTNAME(id)); + if (SNPRINTF_OVERFLOW(result, len)) return(KRB5_KT_NAME_TOOLONG); - strcpy(name, KTNAME(id)); - /* strcpy will NUL-terminate the destination */ - return(0); } diff --git a/src/lib/krb5/keytab/kt_srvtab.c b/src/lib/krb5/keytab/kt_srvtab.c index da09d87a5..4555ca332 100644 --- a/src/lib/krb5/keytab/kt_srvtab.c +++ b/src/lib/krb5/keytab/kt_srvtab.c @@ -248,21 +248,12 @@ krb5_ktsrvtab_get_name(krb5_context context, krb5_keytab id, char *name, unsigne * trt will happen if the name is passed back to resolve. */ { - memset(name, 0, len); - - if (len < strlen(id->ops->prefix)+2) - return(KRB5_KT_NAME_TOOLONG); - strcpy(name, id->ops->prefix); - name += strlen(id->ops->prefix); - name[0] = ':'; - name++; - len -= strlen(id->ops->prefix)+1; + int result; - if (len < strlen(KTFILENAME(id))+1) + memset(name, 0, len); + result = snprintf(name, len, "%s:%s", id->ops->prefix, KTFILENAME(id)); + if (SNPRINTF_OVERFLOW(result, len)) return(KRB5_KT_NAME_TOOLONG); - strcpy(name, KTFILENAME(id)); - /* strcpy will NUL-terminate the destination */ - return(0); } diff --git a/src/lib/krb5/krb/gic_pwd.c b/src/lib/krb5/krb/gic_pwd.c index bd5cbd195..ab491105e 100644 --- a/src/lib/krb5/krb/gic_pwd.c +++ b/src/lib/krb5/krb/gic_pwd.c @@ -45,10 +45,7 @@ krb5_get_as_key_password( if ((ret = krb5_unparse_name(context, client, &clientstr))) return(ret); - strcpy(promptstr, "Password for "); - strncat(promptstr, clientstr, sizeof(promptstr)-strlen(promptstr)-1); - promptstr[sizeof(promptstr)-1] = '\0'; - + snprintf(promptstr, sizeof(promptstr), "Password for %s", clientstr); free(clientstr); prompt.prompt = promptstr; -- 2.26.2