I looked for more instances of the pattern that lead to reading one byte
before an allocated buffer in which(1) when PATH begins with "/:". I
found only one, in the function csexists() in usr.bin/mg/cscope.c.

While at it, I replaced the manual length check before snprintf() with a
check of the return value, and I replaced the space indents in the
csexists() function with tabs. (The rest of the file uses tabs.)
If that makes the diff inconvenient to check, I'll resubmit it without
the indent change.

Index: usr.bin/mg/cscope.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/cscope.c,v
retrieving revision 1.11
diff -u -p -r1.11 cscope.c
--- usr.bin/mg/cscope.c 29 Sep 2015 02:07:49 -0000      1.11
+++ usr.bin/mg/cscope.c 15 Jan 2016 01:38:11 -0000
@@ -592,43 +592,42 @@ csflush(void)
 int
 csexists(const char *cmd)
 {
-       char fname[NFILEN], *dir, *path, *pathc, *tmp;
-       int  cmdlen, dlen;
+       char fname[NFILEN], *dir, *path, *pathc, *tmp;
+       int  len, dlen;
 
-       /* Special case if prog contains '/' */
-       if (strchr(cmd, '/')) {
-               if (access(cmd, F_OK) == -1)
-                       return (FALSE);
-               else
-                       return (TRUE);
-       }
-       if ((tmp = getenv("PATH")) == NULL)
-               return (FALSE);
-       if ((pathc = path = strndup(tmp, NFILEN)) == NULL) {
-               dobeep();
-               ewprintf("out of memory");
-               return (FALSE);
-       }
-       cmdlen = strlen(cmd);
-       while ((dir = strsep(&path, ":")) != NULL) {
-               if (*dir == '\0')
-                       *dir = '.';
+       /* Special case if prog contains '/' */
+       if (strchr(cmd, '/')) {
+               if (access(cmd, F_OK) == -1)
+                       return (FALSE);
+               else
+                       return (TRUE);
+       }
+       if ((tmp = getenv("PATH")) == NULL)
+               return (FALSE);
+       if ((pathc = path = strndup(tmp, NFILEN)) == NULL) {
+               dobeep();
+               ewprintf("out of memory");
+               return (FALSE);
+       }
+       while ((dir = strsep(&path, ":")) != NULL) {
+               if (*dir == '\0')
+                       *dir = '.';
 
-               dlen = strlen(dir);
-               while (dir[dlen-1] == '/')
-                       dir[--dlen] = '\0';     /* strip trailing '/' */
+               dlen = strlen(dir);
+               while (dlen > 0 && dir[dlen-1] == '/')
+                       dir[--dlen] = '\0';     /* strip trailing '/' */
 
-               if (dlen + 1 + cmdlen >= sizeof(fname))  {
-                       dobeep();
-                       ewprintf("path too long");
-                       goto cleanup;
-               }
-               snprintf(fname, sizeof(fname), "%s/%s", dir, cmd);
-               if(access(fname, F_OK) == 0) {
-                      free(pathc);
-                      return (TRUE);
-              }
-       }
+               len = snprintf(fname, sizeof(fname), "%s/%s", dir, cmd);
+               if (len < 0 || len >= sizeof(fname)) {
+                       dobeep();
+                       ewprintf("path too long");
+                       goto cleanup;
+               }
+               if(access(fname, F_OK) == 0) {
+                       free(pathc);
+                       return (TRUE);
+               }
+       }
 cleanup:
        free(pathc);
        return (FALSE);

Reply via email to