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

diff --git a/src/appl/gssftp/ftp/cmds.c b/src/appl/gssftp/ftp/cmds.c
index e733781570c85d98e506ad30043b5960ec0273e3..2f7c8310a621be66d2632f3dca3d9c8e63c4056c 100644 (file)
--- 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 <ctype.h>
 #include <time.h>
 
+#include <k5-platform.h>
+
 #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 485332d3bba8d07daaf42c0849b1f92915f337d7..4405e9b17fbcb2ac4347e60a21b56f40efa4b62f 100644 (file)
--- 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 <libpty.h>
 
+#include <k5-platform.h>
+
 #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 ff94d01126b5e3008d3debc66adc57837a6e2ad4..aec975670e31b448911d0bda6ada2790e81b5645 100644 (file)
--- a/src/appl/telnet/libtelnet/kerberos5.c
+++ b/src/appl/telnet/libtelnet/kerberos5.c
@@ -66,6 +66,7 @@
 #include <errno.h>
 #include <stdio.h>
 #include "krb5.h"
+#include "k5-platform.h"
 
 #include "com_err.h"
 #include <netdb.h>
@@ -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 b3e0e9dfcc990001969c6727eebfea53340b6c2c..b12bd09cfe9749d11c5976a0ed406e063f357d42 100644 (file)
--- a/src/appl/telnet/libtelnet/spx.c
+++ b/src/appl/telnet/libtelnet/spx.c
@@ -71,6 +71,7 @@
 #include <arpa/telnet.h>
 #include <stdio.h>
 #include "gssapi_defs.h"
+#include "k5-platform.h"
 #ifdef __STDC__
 #include <stdlib.h>
 #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 d78c2e83dea6d53ac66f09e17cad6a1a0a1956ad..d86bafd8f240b7412a9c96101d829192161a33bc 100644 (file)
--- 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 ec8f2af0e0c4fa7e8f0fd02bc177929ea8b9ee25..8fd11a3c88eeab04e693e1dd67c694eba4fb3694 100644 (file)
--- 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 4a2b1aef7ac622b3f377ad2fe69d915ca1a4cbac..279d6fd968770e3542ca7f60d18abe1ba01f73b1 100644 (file)
--- 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 649002e21fbcd37aaa1c7679395a735db992a13e..a9106debce1089668972d8a6e2dd85403d74cb34 100644 (file)
--- 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 e1b4c713ce889216773b8e137d8371d421bd9bc1..bdcb329993316639b4668a18d25eeb54cdee6981 100644 (file)
--- 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 d6f724ce0a9a4c338634b6976b4bf85a69cda03b..f7a855b72975f4fcc3b5ce05de64afa4e968f57f 100644 (file)
--- 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 22c01b8b705a312fc5ed7903e8da81a1dba88fa3..b5a099059989cffff83bcdb6cd3f598b582f5d0c 100644 (file)
--- 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 bf394d887547d5cb2c65bd87b3604356e7f455a8..83fb264852eb115ae3d2ec83becd773e84c94107 100644 (file)
--- 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 1b4af9bd6906079ee6219ba542fb335aeb2809f9..53d15edd87fa7d56b9bd2a4b5853a2b016203d23 100644 (file)
--- 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 da09d87a57758712198c91f9d411fab79d7a9476..4555ca33293b93e465fbfd1fde3380f5fd9c91c6 100644 (file)
--- 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 bd5cbd195c8ed10750030d6ef4fd2ba4c6f2e7b3..ab491105e3449121872e95738c1b33fe0cc5bc0b 100644 (file)
--- 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;