From c19ef9a1e4c79a93deed854dfb70e89fac71e3d3 Mon Sep 17 00:00:00 2001 From: asomers Date: Mon, 1 Oct 2018 16:09:20 +0000 Subject: [PATCH] MFC r336609: Fix several Coverity warnings in tftp Some of the changes are in the libexec/tftpd directory, but to functions that are only used by tftp(1) (they share some code). * strcpy => strlcpy (1006793, 1006794, 1006796, 1006741) * Unchecked return value and TOCTTOU (1009314) * NULL pointer dereference (1018035, 1018036) Reported by: Coverity CID: 1006793, 1006794, 1006796, 1006741, 1009314, 1018035 CID: 1018036 git-svn-id: svn://svn.freebsd.org/base/stable/10@339060 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- libexec/tftpd/tftp-io.c | 13 +++++++------ libexec/tftpd/tftp-utils.c | 7 ++++--- usr.bin/tftp/main.c | 12 +++++++++--- usr.bin/tftp/tftp.c | 3 +++ 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/libexec/tftpd/tftp-io.c b/libexec/tftpd/tftp-io.c index 6a6800ffc..e28077e5e 100644 --- a/libexec/tftpd/tftp-io.c +++ b/libexec/tftpd/tftp-io.c @@ -38,6 +38,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -191,16 +192,16 @@ send_wrq(int peer, char *filename, char *mode) tp = (struct tftphdr *)buf; tp->th_opcode = htons((u_short)WRQ); - size = 2; + size = offsetof(struct tftphdr, th_stuff); bp = tp->th_stuff; - strcpy(bp, filename); + strlcpy(bp, filename, sizeof(buf) - size); bp += strlen(filename); *bp = 0; bp++; size += strlen(filename) + 1; - strcpy(bp, mode); + strlcpy(bp, mode, sizeof(buf) - size); bp += strlen(mode); *bp = 0; bp++; @@ -239,16 +240,16 @@ send_rrq(int peer, char *filename, char *mode) tp = (struct tftphdr *)buf; tp->th_opcode = htons((u_short)RRQ); - size = 2; + size = offsetof(struct tftphdr, th_stuff); bp = tp->th_stuff; - strcpy(bp, filename); + strlcpy(bp, filename, sizeof(buf) - size); bp += strlen(filename); *bp = 0; bp++; size += strlen(filename) + 1; - strcpy(bp, mode); + strlcpy(bp, mode, sizeof(buf) - size); bp += strlen(mode); *bp = 0; bp++; diff --git a/libexec/tftpd/tftp-utils.c b/libexec/tftpd/tftp-utils.c index 25a305ffd..6539fb646 100644 --- a/libexec/tftpd/tftp-utils.c +++ b/libexec/tftpd/tftp-utils.c @@ -235,14 +235,15 @@ const char * debug_show(int d) { static char s[100]; + size_t space = sizeof(s); int i = 0; s[0] = '\0'; while (debugs[i].name != NULL) { if (d&debugs[i].value) { - if (s[0] != '\0') - strcat(s, " "); - strcat(s, debugs[i].name); + if (s[0] != '\0') + strlcat(s, " ", space); + strlcat(s, debugs[i].name, space); } i++; } diff --git a/usr.bin/tftp/main.c b/usr.bin/tftp/main.c index 49bd62a4a..b1b24643d 100644 --- a/usr.bin/tftp/main.c +++ b/usr.bin/tftp/main.c @@ -405,7 +405,7 @@ static void settftpmode(const char *newmode) { - strcpy(mode, newmode); + strlcpy(mode, newmode, sizeof(mode)); if (verbose) printf("mode set to %s\n", mode); } @@ -465,7 +465,10 @@ put(int argc, char *argv[]) return; } - stat(cp, &sb); + if (fstat(fd, &sb) < 0) { + warn("%s", cp); + return; + } asprintf(&options[OPT_TSIZE].o_request, "%ju", sb.st_size); if (verbose) @@ -486,7 +489,10 @@ put(int argc, char *argv[]) continue; } - stat(cp, &sb); + if (fstat(fd, &sb) < 0) { + warn("%s", argv[n]); + continue; + } asprintf(&options[OPT_TSIZE].o_request, "%ju", sb.st_size); if (verbose) diff --git a/usr.bin/tftp/tftp.c b/usr.bin/tftp/tftp.c index 6514af3d2..ad2189156 100644 --- a/usr.bin/tftp/tftp.c +++ b/usr.bin/tftp/tftp.c @@ -48,6 +48,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include @@ -83,6 +84,7 @@ xmitfile(int peer, char *port, int fd, char *name, char *mode) if (port == NULL) { struct servent *se; se = getservbyname("tftp", "udp"); + assert(se != NULL); ((struct sockaddr_in *)&peer_sock)->sin_port = se->s_port; } else ((struct sockaddr_in *)&peer_sock)->sin_port = @@ -182,6 +184,7 @@ recvfile(int peer, char *port, int fd, char *name, char *mode) if (port == NULL) { struct servent *se; se = getservbyname("tftp", "udp"); + assert(se != NULL); ((struct sockaddr_in *)&peer_sock)->sin_port = se->s_port; } else ((struct sockaddr_in *)&peer_sock)->sin_port = -- 2.42.0