From c4e3a0c3f0c72c6932d132974e268f0c0a54afbc Mon Sep 17 00:00:00 2001 From: David Levine Date: Sat, 24 Mar 2012 09:13:47 -0500 Subject: [PATCH] Ensure that escape_display_name() can't overrun a buffer. --- h/prototypes.h | 2 +- sbr/escape_display_name.c | 61 ++++++++++++++++++++++++--------------------- sbr/mts.c | 2 +- test/format/test-myname | 3 +++ test/getfullname.c | 11 ++++---- 5 files changed, 43 insertions(+), 36 deletions(-) diff --git a/h/prototypes.h b/h/prototypes.h index b4c0048..2a944e9 100644 --- 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 --- 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 #include #include -#include #include -/* 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 --- 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 --- 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 --- a/test/getfullname.c +++ b/test/getfullname.c @@ -14,14 +14,13 @@ #include #include -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); } -- 1.7.10.4