From 8146bc09fc2e8ad69520ba998be51fb02cbf069c Mon Sep 17 00:00:00 2001 From: Ruud de Rooij Date: Mon, 28 Feb 2000 17:09:02 +0000 Subject: [PATCH] * Fix security hole in mhshowsbr.c which allowed untrusted shell code to be executed. * Released nmh 1.0.3. --- ChangeLog | 6 +++ VERSION | 2 +- configure | 2 +- uip/mhshowsbr.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 111 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9b5ce64..e44eda5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +Sun Feb 20 12:17:15 2000 Ruud de Rooij + + * Fix security hole in mhshowsbr.c which allowed untrusted shell + code to be executed. + * Released nmh 1.0.3. + Thu Feb 10 10:54:36 2000 Dan Harkless * Oops. %-escapes on mhstore lines in mhn.defaults.sh should not diff --git a/VERSION b/VERSION index f650172..21e8796 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.0.2+dev +1.0.3 diff --git a/configure b/configure index 8c9e780..357e2ef 100755 --- a/configure +++ b/configure @@ -1030,7 +1030,7 @@ else int main() { /* Ultrix mips cc rejects this. */ -typedef int charset[2]; const charset x; +typedef int charset[2]; const charset x = {0,0}; /* SunOS 4.1.1 cc rejects this. */ char const *const *ccp; char **p; diff --git a/uip/mhshowsbr.c b/uip/mhshowsbr.c index ef6f02c..9745504 100644 --- a/uip/mhshowsbr.c +++ b/uip/mhshowsbr.c @@ -325,9 +325,9 @@ show_content (CT ct, int serial, int alternate) int show_content_aux (CT ct, int serial, int alternate, char *cp, char *cracked) { - int fd, len, buflen; + int fd, len, buflen, quoted; int xstdin, xlist, xpause, xtty; - char *bp, *file, buffer[BUFSIZ]; + char *bp, *pp, *file, buffer[BUFSIZ]; CI ci = &ct->c_ctinfo; if (!ct->c_ceopenfnx) { @@ -355,12 +355,15 @@ show_content_aux (CT ct, int serial, int alternate, char *cp, char *cracked) /* get buffer ready to go */ bp = buffer; - bp[0] = '\0'; - buflen = sizeof(buffer); + buflen = sizeof(buffer) - 1; + bp[0] = bp[buflen] = '\0'; + quoted = 0; /* Now parse display string */ - for ( ; *cp; cp++) { + for ( ; *cp && buflen > 0; cp++) { if (*cp == '%') { + pp = bp; + switch (*++cp) { case 'a': /* insert parameters from Content-Type field */ @@ -433,14 +436,56 @@ show_content_aux (CT ct, int serial, int alternate, char *cp, char *cracked) len = strlen (bp); bp += len; buflen -= len; + + /* Did we actually insert something? */ + if (bp != pp) { + /* Insert single quote if not inside quotes already */ + if (!quoted && buflen) { + len = strlen (pp); + memmove (pp + 1, pp, len); + *pp++ = '\''; + buflen--; + bp++; + } + /* Escape existing quotes */ + while ((pp = strchr (pp, '\'')) && buflen > 3) { + len = strlen (pp++); + memmove (pp + 3, pp, len); + *pp++ = '\\'; + *pp++ = '\''; + *pp++ = '\''; + buflen -= 3; + bp += 3; + } + /* If pp is still set, that means we ran out of space. */ + if (pp) + buflen = 0; + if (!quoted && buflen) { + *bp++ = '\''; + *bp = '\0'; + buflen--; + } + } } else { raw: - *bp++ = *cp; - *bp = '\0'; - buflen--; + *bp++ = *cp; + *bp = '\0'; + buflen--; + + if (*cp == '\'') + quoted = !quoted; } } + if (buflen <= 0 || (ct->c_termproc && buflen <= strlen(ct->c_termproc))) { + /* content_error would provide a more useful error message + * here, except that if we got overrun, it probably would + * too. + */ + fprintf(stderr, "Buffer overflow constructing show command!\n"); + return NOTOK; + } + /* use charset string to modify display method */ if (ct->c_termproc) { char term[BUFSIZ]; @@ -782,9 +827,9 @@ out: static int show_multi_aux (CT ct, int serial, int alternate, char *cp) { - int len, buflen; + int len, buflen, quoted; int xlist, xpause, xtty; - char *bp, *file, buffer[BUFSIZ]; + char *bp, *pp, *file, buffer[BUFSIZ]; struct multipart *m = (struct multipart *) ct->c_ctparams; struct part *part; CI ci = &ct->c_ctinfo; @@ -819,8 +864,9 @@ show_multi_aux (CT ct, int serial, int alternate, char *cp) /* get buffer ready to go */ bp = buffer; - bp[0] = '\0'; - buflen = sizeof(buffer); + buflen = sizeof(buffer) - 1; + bp[0] = bp[buflen] = '\0'; + quoted = 0; /* Now parse display string */ for ( ; *cp; cp++) { @@ -908,14 +954,56 @@ show_multi_aux (CT ct, int serial, int alternate, char *cp) len = strlen (bp); bp += len; buflen -= len; + + /* Did we actually insert something? */ + if (bp != pp) { + /* Insert single quote if not inside quotes already */ + if (!quoted && buflen) { + len = strlen (pp); + memmove (pp + 1, pp, len); + *pp++ = '\''; + buflen--; + bp++; + } + /* Escape existing quotes */ + while ((pp = strchr (pp, '\'')) && buflen > 3) { + len = strlen (pp++); + memmove (pp + 3, pp, len); + *pp++ = '\\'; + *pp++ = '\''; + *pp++ = '\''; + buflen -= 3; + bp += 3; + } + /* If pp is still set, that means we ran out of space. */ + if (pp) + buflen = 0; + if (!quoted && buflen) { + *bp++ = '\''; + *bp = '\0'; + buflen--; + } + } } else { raw: - *bp++ = *cp; - *bp = '\0'; - buflen--; + *bp++ = *cp; + *bp = '\0'; + buflen--; + + if (*cp == '\'') + quoted = !quoted; } } + if (buflen <= 0 || (ct->c_termproc && buflen <= strlen(ct->c_termproc))) { + /* content_error would provide a more useful error message + * here, except that if we got overrun, it probably would + * too. + */ + fprintf(stderr, "Buffer overflow constructing show command!\n"); + return NOTOK; + } + /* use charset string to modify display method */ if (ct->c_termproc) { char term[BUFSIZ]; -- 1.7.10.4