From: Tom Yu Date: Thu, 26 Apr 2001 00:55:20 +0000 (+0000) Subject: * ftpcmd.y: Don't dereference a NULL pointer returned from X-Git-Tag: krb5-1.3-alpha1~1553 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=9ea22ba225059c4c1cfabf073338273c3e88bb7e;p=krb5.git * ftpcmd.y: Don't dereference a NULL pointer returned from ftpglob(). * ftpd.c: Be more paranoid about return values from ftpglob(). Police uses of sprintf(). Account for expansion in radix_encode(). git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@13193 dc483132-0cff-0310-8789-dd5450dbe970 --- diff --git a/src/appl/gssftp/ftpd/ChangeLog b/src/appl/gssftp/ftpd/ChangeLog index 8ea2c7222..4da4317da 100644 --- a/src/appl/gssftp/ftpd/ChangeLog +++ b/src/appl/gssftp/ftpd/ChangeLog @@ -1,3 +1,12 @@ +2001-04-25 Tom Yu + + * ftpcmd.y: Don't dereference a NULL pointer returned from + ftpglob(). + + * ftpd.c: Be more paranoid about return values from ftpglob(). + Police uses of sprintf(). Account for expansion in + radix_encode(). + 2001-03-07 Ken Raeburn * ftpd.c (strerror): Only define if not HAVE_STRERROR. diff --git a/src/appl/gssftp/ftpd/ftpcmd.y b/src/appl/gssftp/ftpd/ftpcmd.y index 197815a6f..1c5ac1e0c 100644 --- a/src/appl/gssftp/ftpd/ftpcmd.y +++ b/src/appl/gssftp/ftpd/ftpcmd.y @@ -805,11 +805,15 @@ pathname: pathstring * This is a valid reply in some cases but not in others. */ if (logged_in && $1 && strncmp((char *) $1, "~", 1) == 0) { - *(char **)&($$) = *ftpglob((char *) $1); - if (globerr != NULL) { + char **vv; + + vv = ftpglob((char *) $1); + if (vv == NULL || globerr != NULL) { reply(550, globerr); $$ = NULL; - } + } else + $$ = *vv; + free((char *) $1); } else $$ = $1; diff --git a/src/appl/gssftp/ftpd/ftpd.c b/src/appl/gssftp/ftpd/ftpd.c index 0cd613e53..717fdd09a 100644 --- a/src/appl/gssftp/ftpd/ftpd.c +++ b/src/appl/gssftp/ftpd/ftpd.c @@ -763,7 +763,17 @@ user(name) int result; #ifdef GSSAPI if (auth_type && strcmp(auth_type, "GSSAPI") == 0) { + int len; + authorized = ftpd_gss_userok(&client_name, name) == 0; + len = sizeof("GSSAPI user is not authorized as " + "; Password required.") + + strlen(client_name.value) + + strlen(name); + if (len >= sizeof(buf)) { + syslog(LOG_ERR, "user: username too long"); + name = "[username too long]"; + } sprintf(buf, "GSSAPI user %s is%s authorized as %s", client_name.value, authorized ? "" : " not", name); @@ -774,7 +784,19 @@ user(name) #endif /* GSSAPI */ #ifdef KRB5_KRB4_COMPAT if (auth_type && strcmp(auth_type, "KERBEROS_V4") == 0) { + int len; + authorized = kuserok(&kdata,name) == 0; + len = sizeof("Kerberos user .@ is not authorized as " + "; Password required.") + + strlen(kdata.pname) + + strlen(kdata.pinst) + + strlen(kdata.prealm) + + strlen(name); + if (len >= sizeof(buf)) { + syslog(LOG_ERR, "user: username too long"); + name = "[username too long]"; + } sprintf(buf, "Kerberos user %s%s%s@%s is%s authorized as %s", kdata.pname, *kdata.pinst ? "." : "", kdata.pinst, kdata.prealm, @@ -1181,6 +1203,11 @@ retrieve(cmd, name) } else { char line[FTP_BUFSIZ]; + if (strlen(cmd) + strlen(name) + 1 >= sizeof(line)) { + syslog(LOG_ERR, "retrieve: filename too long"); + reply(501, "filename too long"); + return; + } (void) sprintf(line, cmd, name), name = line; fin = ftpd_popen(line, "r"), closefunc = ftpd_pclose; st.st_size = -1; @@ -1419,6 +1446,10 @@ dataconn(name, size, mode) return (file); } +/* + * XXX callers need to limit total length of output string to + * FTP_BUFSIZ + */ #ifdef STDARG secure_error(char *fmt, ...) #else @@ -1618,13 +1649,19 @@ statfilecmd(filename) { char line[FTP_BUFSIZ]; FILE *fin; - int c; + int c, n; char str[FTP_BUFSIZ], *p; + if (strlen(filename) + sizeof("/bin/ls -lgA ") + >= sizeof(line)) { + reply(501, "filename too long"); + return; + } (void) sprintf(line, "/bin/ls -lgA %s", filename); fin = ftpd_popen(line, "r"); lreply(211, "status of %s:", filename); p = str; + n = 0; while ((c = getc(fin)) != EOF) { if (c == '\n') { if (ferror(stdout)){ @@ -1641,7 +1678,16 @@ statfilecmd(filename) *p = '\0'; reply(0, "%s", str); p = str; - } else *p++ = c; + n = 0; + } else { + *p++ = c; + n++; + if (n >= sizeof(str)) { + reply(551, "output line too long"); + (void) ftpd_pclose(fin); + return; + } + } } if (p != str) { *p = '\0'; @@ -1725,6 +1771,10 @@ fatal(s) char cont_char = ' '; +/* + * XXX callers need to limit total length of output string to + * FTP_BUFSIZ bytes for now. + */ #ifdef STDARG reply(int n, char *fmt, ...) #else @@ -1746,22 +1796,32 @@ reply(n, fmt, p0, p1, p2, p3, p4, p5) #endif if (auth_type) { - char in[FTP_BUFSIZ], out[FTP_BUFSIZ]; + /* + * Deal with expansion in mk_{safe,priv}, + * radix_encode, gss_seal, plus slop. + */ + char in[FTP_BUFSIZ*3/2], out[FTP_BUFSIZ*3/2]; int length, kerror; if (n) sprintf(in, "%d%c", n, cont_char); else in[0] = '\0'; strncat(in, buf, sizeof (in) - strlen(in) - 1); #ifdef KRB5_KRB4_COMPAT if (strcmp(auth_type, "KERBEROS_V4") == 0) { - if ((length = clevel == PROT_P ? - krb_mk_priv((unsigned char *)in, - (unsigned char *)out, - strlen(in), schedule, &kdata.session, - &ctrl_addr, &his_addr) - : krb_mk_safe((unsigned char *)in, - (unsigned char *)out, - strlen(in), &kdata.session, - &ctrl_addr, &his_addr)) == -1) { + if (clevel == PROT_P) + length = krb_mk_priv((unsigned char *)in, + (unsigned char *)out, + strlen(in), + schedule, &kdata.session, + &ctrl_addr, + &his_addr); + else + length = krb_mk_safe((unsigned char *)in, + (unsigned char *)out, + strlen(in), + &kdata.session, + &ctrl_addr, + &his_addr); + if (length == -1) { syslog(LOG_ERR, "krb_mk_%s failed for KERBEROS_V4", clevel == PROT_P ? "priv" : "safe"); @@ -1805,13 +1865,16 @@ reply(n, fmt, p0, p1, p2, p3, p4, p5) } #endif /* GSSAPI */ /* Other auth types go here ... */ - if (kerror = radix_encode(out, in, &length, 0)) { + if (length >= sizeof(in) / 4 * 3) { + syslog(LOG_ERR, "input to radix_encode too long"); + fputs(in, stdout); + } else if (kerror = radix_encode(out, in, &length, 0)) { syslog(LOG_ERR, "Couldn't encode reply (%s)", radix_error(kerror)); fputs(in,stdout); } else - printf("%s%c%s", clevel == PROT_P ? "632" : "631", - n ? cont_char : '-', in); + printf("%s%c%s", clevel == PROT_P ? "632" : "631", + n ? cont_char : '-', in); } else { if (n) printf("%d%c", n, cont_char); fputs(buf, stdout); @@ -1824,6 +1887,10 @@ reply(n, fmt, p0, p1, p2, p3, p4, p5) } } +/* + * XXX callers need to limit total length of output string to + * FTP_BUFSIZ + */ #ifdef STDARG lreply(int n, char *fmt, ...) #else @@ -1868,7 +1935,8 @@ yyerror(s) if (cp = strchr(cbuf,'\n')) *cp = '\0'; - reply(500, "'%s': command not understood.", cbuf); + reply(500, "'%.*s': command not understood.", + FTP_BUFSIZ - sizeof("'': command not understood."), cbuf); } delete_file(name) @@ -2145,7 +2213,23 @@ perror_reply(code, string) int code; char *string; { - reply(code, "%s: %s.", string, strerror(errno)); + char *err_string; + size_t extra_len; + + err_string = strerror(errno); + if (err_string == NULL) + err_string = "(unknown error)"; + extra_len = strlen(err_string) + sizeof("(truncated): ."); + + /* + * XXX knows about FTP_BUFSIZ in reply() + */ + if (strlen(string) + extra_len > FTP_BUFSIZ) { + reply(code, "(truncated)%.*s: %s.", + FTP_BUFSIZ - extra_len, string, err_string); + } else { + reply(code, "%s: %s.", string, err_string); + } } auth(type) @@ -2227,6 +2311,10 @@ char *data; secure_error("ADAT: krb_mk_safe failed"); return(0); } + if (length >= (FTP_BUFSIZ - sizeof("ADAT=")) / 4 * 3) { + secure_error("ADAT: reply too long"); + return(0); + } if (kerror = radix_encode(out_buf, buf, &length, 0)) { secure_error("Couldn't encode ADAT reply (%s)", radix_error(kerror)); @@ -2361,6 +2449,16 @@ char *data; } if (out_tok.length) { + if (out_tok.length >= ((FTP_BUFSIZ - sizeof("ADAT=")) + / 4 * 3)) { + secure_error("ADAT: reply too long"); + syslog(LOG_ERR, "ADAT: reply too long"); + (void) gss_release_cred(&stat_min, &server_creds); + if (ret_flags & GSS_C_DELEG_FLAG) + (void) gss_release_cred(&stat_min, + &deleg_creds); + return(0); + } if (kerror = radix_encode(out_tok.value, gbuf, &out_tok.length, 0)) { secure_error("Couldn't encode ADAT reply (%s)", radix_error(kerror)); @@ -2459,6 +2557,9 @@ static char *onefile[] = { * n>=0 on success * -1 on error * -2 on security error + * + * XXX callers need to limit total length of output string to + * FTP_BUFSIZ */ #ifdef STDARG secure_fprintf(FILE *stream, char *fmt, ...) @@ -2576,6 +2677,15 @@ send_file_list(whichfiles) dir->d_name[2] == '\0') continue; + if (strlen(dirname) + strlen(dir->d_name) + + 1 /* slash */ + + 2 /* CRLF */ + + 1 > sizeof(nbuf)) { + syslog(LOG_ERR, + "send_file_list: pathname too long"); + ret = -2; /* XXX */ + goto data_err; + } sprintf(nbuf, "%s/%s", dirname, dir->d_name); /*