From 32cc11aa10796d2c7ed995a53c31ff376a707aa9 Mon Sep 17 00:00:00 2001 From: millert Date: Tue, 16 Dec 2014 18:31:06 +0000 Subject: [PATCH] Use glob() to expand filenames instead of passing it to the shell's echo command for expansion which could result in arbitrary command execution. CVE-2004-2771 --- usr.bin/mail/fio.c | 75 ++++++++++++++++------------------------------ 1 file changed, 26 insertions(+), 49 deletions(-) diff --git a/usr.bin/mail/fio.c b/usr.bin/mail/fio.c index 205ce36ed53..fef47870ed2 100644 --- a/usr.bin/mail/fio.c +++ b/usr.bin/mail/fio.c @@ -1,4 +1,4 @@ -/* $OpenBSD: fio.c,v 1.33 2014/01/17 18:42:30 okan Exp $ */ +/* $OpenBSD: fio.c,v 1.34 2014/12/16 18:31:06 millert Exp $ */ /* $NetBSD: fio.c,v 1.8 1997/07/07 22:57:55 phil Exp $ */ /* @@ -37,6 +37,7 @@ #include #include #include +#include #include "extern.h" /* @@ -412,14 +413,11 @@ fsize(FILE *iob) char * expand(char *name) { + const int flags = GLOB_BRACE|GLOB_TILDE|GLOB_NOSORT; char xname[PATHSIZE]; char cmdbuf[PATHSIZE]; /* also used for file names */ - pid_t pid; - int l; - char *cp, *shell; - int pivec[2]; - struct stat sbuf; - extern int wait_status; + char *match = NULL; + glob_t names; /* * The order of evaluation is "%" and "#" expand into constants. @@ -453,50 +451,29 @@ expand(char *name) (void)snprintf(xname, sizeof(xname), "%s%s", homedir, name + 1); name = savestr(xname); } - if (strpbrk(name, "~{[*?$`'\"\\") == NULL) - return(name); - /* XXX - just use glob(3) and env expansion instead? */ - if (pipe(pivec) < 0) { - warn("pipe"); - return(name); - } - (void)snprintf(cmdbuf, sizeof(cmdbuf), "echo %s", name); - shell = value("SHELL"); - pid = start_command(shell, 0, -1, pivec[1], "-c", cmdbuf, NULL); - if (pid < 0) { - (void)close(pivec[0]); - (void)close(pivec[1]); - return(NULL); - } - (void)close(pivec[1]); - l = myread(pivec[0], xname, PATHSIZE); - if (l < 0) - warn("read"); /* report error before errno changes */ - (void)close(pivec[0]); - if (wait_child(pid) < 0 && WIFSIGNALED(wait_status) && - WTERMSIG(wait_status) != SIGPIPE) { - fprintf(stderr, "\"%s\": Expansion failed.\n", name); - return(NULL); - } - if (l < 0) - return(NULL); - if (l == 0) { + if (strpbrk(name, "~{[*?\\") == NULL) + return(savestr(name)); + + /* XXX - does not expand enviroment variables. */ + switch (glob(name, flags, NULL, &names)) { + case 0: + if (names.gl_pathc == 1) + match = savestr(names.gl_pathv[0]); + else + fprintf(stderr, "\"%s\": Ambiguous.\n", name); + break; + case GLOB_NOSPACE: + fprintf(stderr, "\"%s\": Out of memory.\n", name); + break; + case GLOB_NOMATCH: fprintf(stderr, "\"%s\": No match.\n", name); - return(NULL); - } - if (l == PATHSIZE) { - fprintf(stderr, "\"%s\": Expansion buffer overflow.\n", name); - return(NULL); - } - xname[l] = '\0'; - for (cp = &xname[l-1]; *cp == '\n' && cp > xname; cp--) - ; - cp[1] = '\0'; - if (strchr(xname, ' ') && stat(xname, &sbuf) < 0) { - fprintf(stderr, "\"%s\": Ambiguous.\n", name); - return(NULL); + break; + default: + fprintf(stderr, "\"%s\": Expansion failed.\n", name); + break; } - return(savestr(xname)); + globfree(&names); + return(match); } /* -- 2.20.1