Ensure that escape_display_name() can't overrun a buffer.

diff --git a/h/prototypes.h b/h/prototypes.h
index b4c0048..2a944e9 100644 (file)
--- a/h/prototypes.h
+++ b/h/prototypes.h
@@ -47,7 +47,7 @@ void cpydgst (int, int, char *, char *);
 int decode_rfc2047 (char *, char *, size_t);
 void discard (FILE *);
 int default_done (int);
-void escape_display_name (char *);
+void escape_display_name (char *, size_t);
 int ext_hook(char *, char *, char *);
 int fdcompare (int, int);
 int folder_addmsg (struct msgs **, char *, int, int, int, int, char *);

diff --git a/sbr/escape_display_name.c b/sbr/escape_display_name.c
index 0918942..4e98e97 100644 (file)
--- a/sbr/escape_display_name.c
+++ b/sbr/escape_display_name.c
@@ -1,63 +1,68 @@
+/*
+ * escape_display_name.c -- Escape a display name, hopefully per RFC 5322.
+ *
+ * This code is Copyright (c) 2012, by the authors of nmh.  See the
+ * COPYRIGHT file in the root directory of the nmh distribution for
+ * complete copyright information.
+ */
+
 #include <sys/types.h>
 #include <h/utils.h>
 #include <string.h>
-#include <stdio.h>
 #include <stdlib.h>
 
-/* Escape a display name, hopefully per RFC 5322.
-   The argument is assumed to be a pointer to a character array of
-   one-byte characters with enough space to handle the additional
-   characters. */
+/* Escape a display name, hopefully per RFC 5322.  Assumes one-byte
+   characters.  The char array pointed to by the name argument is
+   modified in place.  Its size is specified by the namesize
+   argument. */
 void
-escape_display_name (char *name) {
+escape_display_name (char *name, size_t namesize) {
     /* Quote and escape name that contains any specials, as necessary. */
     if (strpbrk("\"(),.:;<>@[\\]", name)) {
-        size_t len = strlen(name);
         char *destp, *srcp;
-        size_t destpos, srcpos;
-        /* E.g., 2 characters, "", would require 7, "\"\""\0. */
-       char *tmp = malloc (2*len+3);
-
-        for (destp = tmp, srcp = name, destpos = 0, srcpos = 0;
-             *srcp;
-             ++destp, ++srcp, ++destpos, ++srcpos) {
-            if (srcpos == 0) {
+        /* Maximum space requirement would be if each character had
+           to be escaped, plus enclosing double quotes, plus null termintor.
+           E.g., 2 characters, "", would require 7, "\"\""0, where that 0
+           is '\0'. */
+        char *tmp = mh_xmalloc (2*strlen(name) + 3);
+
+        for (destp = tmp, srcp = name; *srcp; ++srcp) {
+            if (srcp == name) {
                 /* Insert initial double quote, if needed. */
                 if (*srcp != '"') {
                     *destp++ = '"';
-                    ++destpos;
                 }
             } else {
-                /* Escape embedded, unescaped ". */
-                if (*srcp == '"'  &&  srcpos < len - 1  &&  *(srcp-1) != '\\') {
+                /* Escape embedded, unescaped double quote. */
+                if (*srcp == '"' && *(srcp+1) != '\0' && *(srcp-1) != '\\') {
                     *destp++ = '\\';
-                    ++destpos;
                 }
             }
 
-            *destp = *srcp;
+            *destp++ = *srcp;
 
             /* End of name. */
-            if (srcpos == len - 1) {
+            if (*(srcp+1) == '\0') {
                 /* Insert final double quote, if needed. */
                 if (*srcp != '"') {
-                    *++destp = '"';
-                    ++destpos;
+                    *destp++ = '"';
                 }
 
-                *++destp = '\0';
-                ++destpos;
+                *destp++ = '\0';
             }
         }
 
         if (strcmp (tmp, "\"")) {
-            /* assert (strlen(tmp) + 1 == destpos); */
-            strncpy (name, tmp, destpos);
+            /* assert (strlen(tmp) + 1 == destp - tmp); */
+            size_t len = destp - tmp;
+            strncpy (name, tmp, len <= namesize  ?  len  :  namesize);
         } else {
             /* Handle just " as special case here instead of above. */
-            strcpy (name, "\"\\\"\"");
+            strncpy (name, "\"\\\"\"", namesize);
         }
 
+        name[namesize - 1] = '\0';
+
         free (tmp);
     }
 }

diff --git a/sbr/mts.c b/sbr/mts.c
index 556d740..b529d15 100644 (file)
--- a/sbr/mts.c
+++ b/sbr/mts.c
@@ -397,7 +397,7 @@ getuserinfo (void)
 
     fullname[sizeof(fullname) - 1] = '\0';
 
-    escape_display_name(fullname);
+    escape_display_name(fullname, sizeof(fullname));
 
     localmbox[0] = '\0';
 

diff --git a/test/format/test-myname b/test/format/test-myname
index d9dca2d..15ca852 100755 (executable)
--- a/test/format/test-myname
+++ b/test/format/test-myname
@@ -36,7 +36,10 @@ escape="${MH_OBJ_DIR}/test/getfullname"
 run_test "$escape "'user'           'user'              'no escape'
 run_test "$escape "'first.last'     '"first.last"'      'escape'
 run_test "$escape "'"first.last"'   '"first.last"'      'already escaped'
+run_test "$escape "'first.last"'    '"first.last"'      'missing initial "'
+run_test "$escape "'"first.last'    '"first.last"'      'missing final "'
 run_test "$escape "'embedded"quote' '"embedded\"quote"' 'embedded quote'
+run_test "$escape "'server\name,#'  '"server\name"'     'Windows form'
 run_test "$escape "'"'              '"\""'              'special "'
 run_test "$escape "'('              '"("'               'special ('
 run_test "$escape "')'              '")"'               'special )'

diff --git a/test/getfullname.c b/test/getfullname.c
index ec83939..0e12fe9 100644 (file)
--- a/test/getfullname.c
+++ b/test/getfullname.c
@@ -14,14 +14,13 @@
 #include <sys/types.h>
 #include <pwd.h>
 
-extern void escape_display_name (char *);
+extern void escape_display_name (char *, size_t);
 
 int
 main(int argc, char *argv[])
 {
        struct passwd *pwd;
        char buf[BUFSIZ], *p;
-       char *name = buf;
 
        if (argc < 2) {
                pwd = getpwuid(getuid());
@@ -35,7 +34,7 @@ main(int argc, char *argv[])
                strncpy(buf, pwd->pw_gecos, sizeof(buf));
                buf[sizeof(buf) - 1] = '\0';
        } else if (argc == 2) {
-               name = argv[1];
+               strncpy(buf, argv[1], sizeof(buf));
        } else if (argc > 2) {
                fprintf (stderr, "usage: %s [name]\n", argv[0]);
                return 1;
@@ -48,15 +47,15 @@ main(int argc, char *argv[])
        /*
         * Stop at the first comma.
         */
-       if ((p = strchr(name, ',')))
+       if ((p = strchr(buf, ',')))
                *p = '\0';
 
        /*
         * Quote the entire string if it has a special character in it.
         */
-       escape_display_name (name);
+       escape_display_name (buf, sizeof(buf));
 
-       printf("%s\n", name);
+       printf("%s\n", buf);
 
        exit(0);
 }