diff options
| -rw-r--r-- | NEWS | 2 | ||||
| -rw-r--r-- | driver.c | 144 | ||||
| -rw-r--r-- | fetchmail.c | 14 | ||||
| -rw-r--r-- | fetchmail.man | 8 |
4 files changed, 104 insertions, 64 deletions
@@ -19,6 +19,8 @@ in addition to the current leading-edge one, until the next gold version. * Marty Lee fixed a bug in activation of hosts named on the command-line. * The fetchall option forces RETR again. We can cope with USA.NET now. * Gunther Leber's patch to make fetchmail -V less chatty when mode is ETRN. +* Gunther Leber's code to sanitize %T and %F expansion in the MDA string. +* Jonathan Marten's fix for list option handling. There are 281 people on fetchmail-friends and 222 on fetchmail-announce. @@ -619,6 +619,19 @@ static int stuffline(struct query *ctl, char *buf) return(n); } + +static void sanitize(s) +/* replace unsafe shellchars by an _ */ +char *s; +{ + static char *ok_chars = " 1234567890!@%-_=+:,./abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; + char *cp; + + for (cp = s; *(cp += strspn(cp, ok_chars)); /* NO INCREMENT */) + *cp = '_'; +} + + #define EMPTYLINE(s) ((s)[0] == '\r' && (s)[1] == '\n' && (s)[2] == '\0') static int readheaders(sock, fetchlen, reallen, ctl, num) @@ -1096,8 +1109,8 @@ int num; /* index of message */ } else if (ctl->mda) /* we have a declared MDA */ { - int length = 0; - char *names, *before, *after; + int length = 0, fromlen = 0, nameslen = 0; + char *names = NULL, *before, *after, *from = NULL; for (idp = xmit_names; idp; idp = idp->next) if (idp->val.status.mark == XMIT_ACCEPT) @@ -1105,32 +1118,22 @@ int num; /* index of message */ destaddr = "localhost"; - length = strlen(ctl->mda) + 1; + length = strlen(ctl->mda); before = xstrdup(ctl->mda); - /* sub user addresses for %T (or %s for backward compatibility) */ - cp = (char *)NULL; - if (strstr(before, "%s") || (cp = strstr(before, "%T"))) + /* get user addresses for %T (or %s for backward compatibility) */ + if (strstr(before, "%s") || strstr(before, "%T")) { - char *sp; - - if (cp && cp[1] == 'T') - cp[1] = 's'; - - /* \177 had better be out-of-band for MDA commands */ - for (sp = before; *sp; sp++) - if (*sp == '%' && sp[1] != 's' && sp[1] != 'T') - *sp = '\177'; - /* * We go through this in order to be able to handle very * long lists of users and (re)implement %s. */ + nameslen = 0; for (idp = xmit_names; idp; idp = idp->next) if (idp->val.status.mark == XMIT_ACCEPT) - length += (strlen(idp->id) + 1); + nameslen += (strlen(idp->id) + 1); /* string + ' ' */ - names = (char *)xmalloc(++length); + names = (char *)xmalloc(nameslen + 1); /* account for '\0' */ names[0] = '\0'; for (idp = xmit_names; idp; idp = idp->next) if (idp->val.status.mark == XMIT_ACCEPT) @@ -1138,48 +1141,82 @@ int num; /* index of message */ strcat(names, idp->id); strcat(names, " "); } - after = (char *)xmalloc(length); -#ifdef SNPRINTF - snprintf(after, length, before, names); -#else - sprintf(after, before, names); -#endif /* SNPRINTF */ - free(names); - free(before); - before = after; - - for (sp = before; *sp; sp++) - if (*sp == '\177') - *sp = '%'; + names[--nameslen] = '\0'; /* chop trailing space */ + + /* sanitize names in order to contain *only* harmless shell chars */ + sanitize(names); } - /* substitute From address for %F */ - if ((cp = strstr(before, "%F"))) + /* get From address for %F */ + if (strstr(before, "%F")) { - char *from = return_path; - char *sp; + from = xstrdup(return_path); - /* \177 had better be out-of-band for MDA commands */ - for (sp = before; *sp; sp++) - if (*sp == '%' && sp[1] != 'F') - *sp = '\177'; + /* sanitize from in order to contain *only* harmless shell chars */ + sanitize(from); - length += strlen(from); - after = (char *)xmalloc(length); - cp[1] = 's'; -#ifdef SNPRINTF - snprintf(after, length, before, from); -#else - sprintf(after, before, from); -#endif /* SNPRINTF */ - free(before); - before = after; - - for (sp = before; *sp; sp++) - if (*sp == '\177') - *sp = '%'; + fromlen = strlen(from); } + /* do we have to build an mda string? */ + if (names || from) { + + char *sp, *dp; + + + /* find length of resulting mda string */ + sp = before; + while (sp = strstr(sp, "%s")) { + length += nameslen - 2; /* subtract %s */ + sp += 2; + } + sp = before; + while (sp = strstr(sp, "%T")) { + length += nameslen - 2; /* subtract %T */ + sp += 2; + } + sp = before; + while (sp = strstr(sp, "%F")) { + length += fromlen - 2; /* subtract %F */ + sp += 2; + } + + after = xmalloc(length + 1); + + /* copy mda source string to after, while expanding %[sTF] */ + for (dp = after, sp = before; *dp = *sp; dp++, sp++) { + if (sp[0] != '%') continue; + + /* need to expand? BTW, no here overflow, because in + ** the worst case (end of string) sp[1] == '\0' */ + if (sp[1] == 's' || sp[1] == 'T') { + strcpy(dp, names); + dp += nameslen; + sp++; /* position sp over [sT] */ + dp--; /* adjust dp */ + } else if (sp[1] == 'F') { + strcpy(dp, from); + dp += fromlen; + sp++; /* position sp over F */ + dp--; /* adjust dp */ + } + } + + if (names) { + free(names); + names = NULL; + } + if (from) { + free(from); + from = NULL; + } + + free(before); + + before = after; + } + + if (outlevel == O_VERBOSE) error(0, 0, "about to deliver with: %s", before); @@ -1195,6 +1232,7 @@ int num; /* index of message */ sinkfp = popen(before, "w"); free(before); + before = NULL; #ifdef HAVE_SETEUID /* this will fail quietly if we didn't start as root */ diff --git a/fetchmail.c b/fetchmail.c index fda397b3..3e3ecf2a 100644 --- a/fetchmail.c +++ b/fetchmail.c @@ -708,13 +708,13 @@ static void optmerge(struct query *h2, struct query *h1, int force) * is nonempty (treat h1 as an override). */ - if (!force) - { - append_str_list(&h2->server.localdomains, &h1->server.localdomains); - append_str_list(&h2->localnames, &h1->localnames); - append_str_list(&h2->mailboxes, &h1->mailboxes); - append_str_list(&h2->smtphunt, &h1->smtphunt); - } +#define LIST_MERGE(fld) if (force ? !!h1->fld : !h2->fld) \ + append_str_list(&h2->fld, &h1->fld) + LIST_MERGE(server.localdomains); + LIST_MERGE(localnames); + LIST_MERGE(mailboxes); + LIST_MERGE(smtphunt); +#undef LIST_MERGE #define FLAG_MERGE(fld) if (force ? !!h1->fld : !h2->fld) h2->fld = h1->fld FLAG_MERGE(server.via); diff --git a/fetchmail.man b/fetchmail.man index bf75889f..11c01026 100644 --- a/fetchmail.man +++ b/fetchmail.man @@ -1588,10 +1588,10 @@ link can be tapped. .PP Use of the %F or %T escapes in an mda option could open a security hole, because they pass text manipulable by an attacker to a shell -command. The hole is reduced by the fact that fetchmail temporarily -discards any suid privileges it may have while running the MDA. To -avoid potential problems, (1) enclose the %F and %T escapes in single -quotes within the option, and (2) never use an mda command containing +command. Potential shell characters are replaced by `_' before +execution. The hole is further reduced by the fact that fetchmail +temporarily discards any suid privileges it may have while running the +MDA. For maximum safety, however, don't use an mda command containing %F or %T when fetchmail is run from the root account itself. .PP Send comments, bug reports, gripes, and the like to Eric S. Raymond |