Use strlc{at,py} for paranoia's sake and exit on overflow. In all

cases the strings were either pre-allocated to the correct size of
length checks were done before the copy but a little paranoia can
go a long way.

diff --git a/check.c b/check.c
index a86c138ffb3e0b4726220f86c0929216366988ad..0e246451078be4d3db3ee18513ae812873b0e96f 100644 (file)
--- a/check.c
+++ b/check.c
@@ -174,9 +174,9 @@ expand_prompt(old_prompt, user, host)
     char *user;
     char *host;
 {
-    size_t len;
+    size_t len, n;
     int subst;
-    char *p, *np, *new_prompt;
+    char *p, *np, *new_prompt, *endp;
 
     /* How much space do we need to malloc for the prompt? */
     subst = 0;
@@ -215,29 +215,35 @@ expand_prompt(old_prompt, user, host)
     }
 
     if (subst) {
-       new_prompt = (char *) emalloc(len + 1);
+       new_prompt = (char *) emalloc(++len);
+       *new_prompt = '\0';
+       endp = new_prompt + len - 1;
        for (p = old_prompt, np = new_prompt; *p; p++) {
            if (p[0] =='%') {
                switch (p[1]) {
                    case 'h':
                        p++;
-                       strcpy(np, user_shost);
-                       np += strlen(user_shost);
+                       if ((n = strlcat(new_prompt, user_shost, len)) >= len)
+                           goto oflow;
+                       np += n;
                        continue;
                    case 'H':
                        p++;
-                       strcpy(np, user_host);
-                       np += strlen(user_host);
+                       if ((n = strlcat(new_prompt, user_host, len)) >= len)
+                           goto oflow;
+                       np += n;
                        continue;
                    case 'u':
                        p++;
-                       strcpy(np, user_name);
-                       np += strlen(user_name);
+                       if ((n = strlcat(new_prompt, user_name, len)) >= len)
+                           goto oflow;
+                       np += n;
                        continue;
                    case 'U':
                        p++;
-                       strcpy(np, *user_runas);
-                       np += strlen(*user_runas);
+                       if ((n = strlcat(new_prompt, *user_runas, len)) >= len)
+                           goto oflow;
+                       np += n;
                        continue;
                    case '%':
                        /* convert %% -> % */
@@ -248,6 +254,8 @@ expand_prompt(old_prompt, user, host)
                        break;
                }
            }
+           if (np >= endp)
+               goto oflow;
            *np++ = *p;
        }
        *np = '\0';
@@ -255,6 +263,12 @@ expand_prompt(old_prompt, user, host)
        new_prompt = old_prompt;
 
     return(new_prompt);
+
+oflow:
+    /* We pre-allocate enough space, so this should never happen. */
+    (void) fprintf(stderr, "%s: internal error, expand_prompt() overflow\n",
+       Argv[0]);
+    exit(1);
 }
 
 /*

diff --git a/env.c b/env.c
index 00ee4d5b6656f6a93a1445c99846cb24706bb306..def9ee6f4ac18308532512789b0faf89a35b04c8 100644 (file)
--- a/env.c
+++ b/env.c
@@ -213,16 +213,20 @@ format_env(var, val)
     char *var;
     char *val;
 {
-    char *estring, *p;
-    size_t varlen, vallen;
-
-    varlen = strlen(var);
-    vallen = strlen(val);
-    p = estring = (char *) emalloc(varlen + vallen + 2);
-    strcpy(p, var);
-    p += varlen;
-    *p++ = '=';
-    strcpy(p, val);
+    char *estring;
+    size_t esize;
+
+    esize = strlen(var) + 1 + strlen(val) + 1;
+    estring = (char *) emalloc(esize);
+
+    /* We pre-allocate enough space, so this should never overflow. */
+    if (strlcpy(estring, var, esize) >= esize ||
+       strlcat(estring, "=", esize) >= esize ||
+       strlcat(estring, val, esize) >= esize) {
+       (void) fprintf(stderr, "%s: internal error, format_env() overflow\n",
+           Argv[0]);
+       exit(1);
+    }
 
     return(estring);
 }

diff --git a/find_path.c b/find_path.c
index 8414e57049de788db5a87ab8a4dc1e52f7c77722..5fbeb4e0ac48041d87011262175da657049ce2a6 100644 (file)
--- a/find_path.c
+++ b/find_path.c
@@ -82,6 +82,7 @@ find_path(infile, outfile, path)
     char *origpath;            /* so we can free path later */
     char *result = NULL;       /* result of path/file lookup */
     int checkdot = 0;          /* check current dir? */
+    int len;                   /* length parameter */
 
     if (strlen(infile) >= MAXPATHLEN) {
        (void) fprintf(stderr, "%s: path too long: %s\n", Argv[0], infile);
@@ -93,7 +94,7 @@ find_path(infile, outfile, path)
      * there is no need to look at $PATH.
      */
     if (strchr(infile, '/')) {
-       (void) strcpy(command, infile);
+       strlcpy(command, infile, sizeof(command));      /* paranoia */
        if (sudo_goodpath(command)) {
            *outfile = command;
            return(FOUND);
@@ -128,11 +129,11 @@ find_path(infile, outfile, path)
        /*
         * Resolve the path and exit the loop if found.
         */
-       if (strlen(path) + strlen(infile) + 1 >= MAXPATHLEN) {
+       len = snprintf(command, sizeof(command), "%s/%s", path, infile);
+       if (len <= 0 || len >= sizeof(command)) {
            (void) fprintf(stderr, "%s: path too long: %s\n", Argv[0], infile);
            exit(1);
        }
-       (void) sprintf(command, "%s/%s", path, infile);
        if ((result = sudo_goodpath(command)))
            break;
 

diff --git a/parse.c b/parse.c
index d0e9bc336b08794b85acc5160ad7e73fcad3f2b5..ec54635fe8d16650972bc8614ea4e9daea0ed5c7 100644 (file)
--- a/parse.c
+++ b/parse.c
@@ -343,10 +343,9 @@ command_matches(cmnd, cmnd_args, path, sudoers_args)
 
        while ((dent = readdir(dirp)) != NULL) {
            /* ignore paths > MAXPATHLEN (XXX - log) */
-           if (plen + NAMLEN(dent) >= sizeof(buf))
+           if (strlcpy(buf, path, sizeof(buf)) >= sizeof(buf) ||
+               strlcat(buf, dent->d_name, sizeof(buf)) >= sizeof(buf))
                continue;
-           strcpy(buf, path);
-           strcat(buf, dent->d_name);
 
            /* only stat if basenames are the same */
            if (strcmp(cmnd_base, dent->d_name) != 0 || stat(buf, &pst) == -1)

diff --git a/parse.yacc b/parse.yacc
index 3f831b0a703034bb34f6cd17701f4cfce9adc9b7..5ed4363381176cb4984a627a34011b70d6e835be 100644 (file)
--- a/parse.yacc
+++ b/parse.yacc
@@ -1109,6 +1109,7 @@ append(src, dstp, dst_len, dst_size, separator)
     /* Assumes dst will be NULL if not set. */
     if (dst == NULL) {
        dst = (char *) emalloc(BUFSIZ);
+       *dst = '\0';
        *dst_size = BUFSIZ;
        *dst_len = 0;
        *dstp = dst;
@@ -1124,12 +1125,10 @@ append(src, dstp, dst_len, dst_size, separator)
     }
 
     /* Copy src -> dst adding a separator if appropriate and adjust len. */
-    dst += *dst_len;
-    *dst_len += src_len;
-    *dst = '\0';
     if (separator)
-       (void) strcat(dst, separator);
-    (void) strcat(dst, src);
+       (void) strlcat(dst, separator, *dst_size);
+    (void) strlcat(dst, src, *dst_size);
+    *dst_len += src_len;
 }
 
 /*

diff --git a/sudo.c b/sudo.c
index 5081845edc1a366a7f3ca4fbd096d47e9b04285d..ac8152659dd290673268bf0f45fe4c9ee901173d 100644 (file)
--- a/sudo.c
+++ b/sudo.c
@@ -480,7 +480,7 @@ init_vars(sudo_mode)
     /* Default value for cmnd and cwd, overridden later. */
     if (user_cmnd == NULL)
        user_cmnd = NewArgv[0];
-    (void) strcpy(user_cwd, "unknown");
+    (void) strlcpy(user_cwd, "unknown", sizeof(user_cwd));
 
     /*
      * We avoid gethostbyname() if possible since we don't want
@@ -556,7 +556,7 @@ init_vars(sudo_mode)
        if (!getcwd(user_cwd, sizeof(user_cwd))) {
            (void) fprintf(stderr, "%s: Can't get working directory!\n",
                           Argv[0]);
-           (void) strcpy(user_cwd, "unknown");
+           (void) strlcpy(user_cwd, "unknown", sizeof(user_cwd));
        }
     } else
        set_perms(PERM_ROOT);
@@ -598,7 +598,7 @@ init_vars(sudo_mode)
        /* set user_args */
        if (NewArgc > 1) {
            char *to, **from;
-           size_t size;
+           size_t size, n;
 
            /* If MODE_SHELL not set then NewArgv is contiguous so just count */
            if (!(sudo_mode & MODE_SHELL)) {
@@ -610,10 +610,15 @@ init_vars(sudo_mode)
            }
 
            /* alloc and copy. */
-           to = user_args = (char *) emalloc(size);
-           for (from = NewArgv + 1; *from; from++) {
-               (void) strcpy(to, *from);
-               to += strlen(*from);
+           user_args = (char *) emalloc(size);
+           for (to = user_args, from = NewArgv + 1; *from; from++) {
+               n = strlcpy(to, *from, size - (to - user_args));
+               if (n >= size) {
+                   (void) fprintf(stderr,
+                       "%s: internal error, init_vars() overflow\n", Argv[0]);
+                   exit(1);
+               }
+               to += n;
                *to++ = ' ';
            }
            *--to = '\0';

diff --git a/testsudoers.c b/testsudoers.c
index fa379456edefe5142273d0d0cc9f8128a0ee2ae8..889f8a96fc0f819d228654a9a8b2c8db45bf6adc 100644 (file)
--- a/testsudoers.c
+++ b/testsudoers.c
@@ -377,17 +377,23 @@ main(argc, argv)
 
     /* Fill in cmnd_args from NewArgv. */
     if (NewArgc > 1) {
-       size_t size;
        char *to, **from;
+       size_t size, n;
 
-       size = (size_t) NewArgv[NewArgc-1] + strlen(NewArgv[NewArgc-1]) -
-              (size_t) NewArgv[1] + 1;
+       size = (size_t) (NewArgv[NewArgc-1] - NewArgv[1]) +
+               strlen(NewArgv[NewArgc-1]) + 1;
        user_args = (char *) emalloc(size);
-       for (to = user_args, from = &NewArgv[1]; *from; from++) {
+       for (to = user_args, from = NewArgv + 1; *from; from++) {
+           n = strlcpy(to, *from, size - (to - user_args));
+           if (n >= size) {
+               (void) fprintf(stderr,
+                   "%s: internal error, init_vars() overflow\n", Argv[0]);
+               exit(1);
+           }
+           to += n;
            *to++ = ' ';
-           (void) strcpy(to, *from);
-           to += strlen(*from);
        }
+       *--to = '\0';
     }
 
     /* Initialize default values. */