Patch to add in missing protection against special characters and malicious servers, backported from netkit ftp 0.17 sources, (There are probably other security fixes that are missing from this old ftp source too). Mark Cox, mjc@redhat.com, Jan 2003 --- krb5-1.2.5/src/appl/gssftp/ftp/cmds.c.ORIG 2003-01-20 10:21:41.000000000 +0000 +++ krb5-1.2.5/src/appl/gssftp/ftp/cmds.c 2003-01-20 11:03:40.000000000 +0000 @@ -69,6 +69,7 @@ extern char **ftpglob(); extern char *home; extern char *remglob(); +static int checkglob(int fd, const char *pattern); extern char *getenv(); #ifndef HAVE_STRERROR #define strerror(error) (sys_errlist[error]) @@ -88,6 +89,64 @@ extern int do_auth(); /* + * pipeprotect: protect against "special" local filenames by prepending + * "./". Special local filenames are "-" and "|..." AND "/...". + */ +static char *pipeprotect(char *name) +{ + char *nu; + if (strcmp(name, "-") && *name!='|' && *name!='/') { + return name; + } + + /* We're going to leak this memory. XXX. */ + nu = malloc(strlen(name)+3); + if (nu==NULL) { + perror("malloc"); + code = -1; + return NULL; + } + strcpy(nu, "."); + if (*name != '/') strcat(nu, "/"); + strcat(nu, name); + return nu; +} + +/* + * Look for embedded ".." in a pathname and change it to "!!", printing + * a warning. + */ +static char *pathprotect(char *name) +{ + int gotdots=0, i, len; + + /* Convert null terminator to trailing / to catch a trailing ".." */ + len = strlen(name)+1; + name[len-1] = '/'; + + /* + * State machine loop. gotdots is < 0 if not looking at dots, + * 0 if we just saw a / and thus might start getting dots, + * and the count of dots seen so far if we have seen some. + */ + for (i=0; i=0) gotdots++; + else if (name[i]=='/' && gotdots<0) gotdots=0; + else if (name[i]=='/' && gotdots==2) { + printf("Warning: embedded .. in %.*s (changing to !!)\n", + len-1, name); + name[i-1] = '!'; + name[i-2] = '!'; + gotdots = 0; + } + else if (name[i]=='/') gotdots = 0; + else gotdots = -1; + } + name[len-1] = 0; + return name; +} + +/* * `Another' gets another argument, and stores the new argc and argv. * It reverts to the top level (via main.c's intr()) on EOF/error. * @@ -832,7 +891,15 @@ if (argc == 2) { argc++; - argv[2] = argv[1]; + /* + * Protect the user from accidentally retrieving special + * local names. + */ + argv[2] = pipeprotect(argv[1]); + if (!argv[2]) { + code = -1; + return 0; + } loc++; } if (argc < 2 && !another(&argc, &argv, "remote-file")) @@ -1007,8 +1074,19 @@ if (mapflag) { tp = domap(tp); } - recvrequest("RETR", tp, cp, "w", - tp != cp || !interactive); + /* Reject embedded ".." */ + tp = pathprotect(tp); + + /* Prepend ./ to "-" or "!*" or leading "/" */ + tp = pipeprotect(tp); + if (tp == NULL) { + /* hmm... how best to handle this? */ + mflag = 0; + } + else { + recvrequest("RETR", tp, cp, "w", + tp != cp || !interactive); + } if (!mflag && fromatty) { ointer = interactive; interactive = 1; @@ -1024,16 +1102,14 @@ } char * -remglob(argv,doswitch) - char *argv[]; - int doswitch; +remglob(char *argv[], int doswitch) { char temp[16]; - static char buf[MAXPATHLEN]; + static char buf[PATH_MAX]; static FILE *ftemp = NULL; static char **args; - int oldverbose, oldhash; - char *cp, *mode; + int oldverbose, oldhash, badglob = 0; + char *cp; if (!mflag) { if (!doglob) { @@ -1055,36 +1131,154 @@ return (cp); } if (ftemp == NULL) { - (void) strncpy(temp, _PATH_TMP, sizeof(temp) - 1); - temp[sizeof(temp) - 1] = '\0'; - (void) mktemp(temp); + int oldumask, fd; + (void) strcpy(temp, _PATH_TMP); + + /* libc 5.2.18 creates with mode 0666, which is dumb */ + oldumask = umask(077); + fd = mkstemp(temp); + umask(oldumask); + + if (fd<0) { + printf("Error creating temporary file, oops\n"); + return NULL; + } + oldverbose = verbose, verbose = 0; oldhash = hash, hash = 0; if (doswitch) { pswitch(!proxy); } - for (mode = "w"; *++argv != NULL; mode = "a") - recvrequest ("NLST", temp, *argv, mode, 0); + while (*++argv != NULL) { + int dupfd = dup(fd); + + recvrequest ("NLST", temp, *argv, "a", 0); + if (!checkglob(dupfd, *argv)) { + badglob = 1; + break; + } + } + unlink(temp); + if (doswitch) { pswitch(!proxy); } verbose = oldverbose; hash = oldhash; - ftemp = fopen(temp, "r"); - (void) unlink(temp); + if (badglob) { + printf("Refusing to handle insecure file list\n"); + close(fd); + return NULL; + } + ftemp = fdopen(fd, "r"); if (ftemp == NULL) { printf("can't find list of remote files, oops\n"); return (NULL); } + rewind(ftemp); } if (fgets(buf, sizeof (buf), ftemp) == NULL) { (void) fclose(ftemp), ftemp = NULL; return (NULL); } - if ((cp = strchr(buf, '\n')) != NULL) + if ((cp = index(buf, '\n')) != NULL) *cp = '\0'; return (buf); } +/* + * Check whether given pattern matches `..' + * We assume only a glob pattern starting with a dot will match + * dot entries on the server. + */ +static int +isdotdotglob(const char *pattern) +{ + int havedot = 0; + char c; + + if (*pattern++ != '.') + return 0; + while ((c = *pattern++) != '\0' && c != '/') { + if (c == '*' || c == '?') + continue; + if (c == '.' && havedot++) + return 0; + } + return 1; +} + +/* + * This function makes sure the list of globbed files returned from + * the server doesn't contain anything dangerous such as + * /home//.forward, or ../.forward, + * or |mail foe@doe = MAXPATHLEN) { + printf("Incredible pattern: %s\n", pattern); + return 0; + } + dotdot[nrslash++] = isdotdotglob(sp); + } + + fp = fdopen(fd, "r"); + while (okay && fgets(buffer, sizeof(buffer), fp) != NULL) { + char *sp; + + if ((sp = strchr(buffer, '\n')) != 0) { + *sp = '\0'; + } else { + printf("Extremely long filename from server: %s", + buffer); + okay = 0; + break; + } + if (buffer[0] == '|' + || (buffer[0] != '/' && initial) + || (buffer[0] == '/' && !initial)) + okay = 0; + for (sp = buffer, nr = 0; sp; sp = strchr(sp, '/'), nr++) { + while (*sp == '/') + sp++; + if (sp[0] == '.' && !strncmp(sp, "../", 3) + && (nr >= nrslash || !dotdot[nr])) + okay = 0; + } + } + + if (!okay) + printf("Filename provided by server " + "doesn't match pattern `%s': %s\n", pattern, buffer); + + fclose(fp); + return okay; +} + char * onoff(bool) int bool;