svn commit: r336605 - head/libexec/tftpd

[Alan Somers]

Author: asomers
Date: Sun Jul 22 16:14:30 2018
New Revision: 336605
URL: https://svnweb.freebsd.org/changeset/base/336605

Log:
  Fix multiple Coverity warnings in tftpd(8)
  
  * Initialize uninitialized variable (CID 1006502)
  * strcpy => strlcpy (CID 1006792, 1006791, 1006790)
  * Check function return values (CID 1009442, 1009441, 1009440)
  * Delete dead code in receive_packet (not reported by Coverity)
  * Remove redundant alarm(3) in receive_packet (not reported by Coverity)
  
  Reported by:	Coverity
  CID: 1006502, 1006792, 1006791, 1006790, 1009442, 1009441, 1009440
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D11287

Modified:
  head/libexec/tftpd/tftp-file.c
  head/libexec/tftpd/tftp-io.c
  head/libexec/tftpd/tftp-utils.c
  head/libexec/tftpd/tftpd.c

Modified: head/libexec/tftpd/tftp-file.c
==============================================================================
--- head/libexec/tftpd/tftp-file.c	Sun Jul 22 14:11:52 2018	(r336604)
+++ head/libexec/tftpd/tftp-file.c	Sun Jul 22 16:14:30 2018	(r336605)
@@ -36,6 +36,7 @@ __FBSDID("$FreeBSD$");
 #include <netinet/in.h>
 #include <arpa/tftp.h>
 
+#include <assert.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -80,7 +81,8 @@ convert_from_net(char *buffer, size_t count)
 		if (buffer[i] == '\n') {
 			if (n == 0) {
 				if (ftell(file) != 0) {
-					fseek(file, -1, SEEK_END);
+					int r = fseek(file, -1, SEEK_END);
+					assert(r == 0);
 					convbuffer[n++] = '\n';
 				} else {
 					/* This shouldn't happen */

Modified: head/libexec/tftpd/tftp-io.c
==============================================================================
--- head/libexec/tftpd/tftp-io.c	Sun Jul 22 14:11:52 2018	(r336604)
+++ head/libexec/tftpd/tftp-io.c	Sun Jul 22 16:14:30 2018	(r336605)
@@ -36,6 +36,7 @@ __FBSDID("$FreeBSD$");
 #include <arpa/tftp.h>
 #include <arpa/inet.h>
 
+#include <assert.h>
 #include <errno.h>
 #include <setjmp.h>
 #include <signal.h>
@@ -395,7 +396,7 @@ receive_packet(int peer, char *data, int size, struct 
 	struct sockaddr_storage *pfrom;
 	socklen_t fromlen;
 	int n;
-	static int waiting;
+	static int timed_out;
 
 	if (debug&DEBUG_PACKETS)
 		tftp_log(LOG_DEBUG,
@@ -403,23 +404,16 @@ receive_packet(int peer, char *data, int size, struct 
 
 	pkt = (struct tftphdr *)data;
 
-	waiting = 0;
 	signal(SIGALRM, timeout);
-	setjmp(timeoutbuf);
+	timed_out = setjmp(timeoutbuf);
 	alarm(thistimeout);
 
-	if (waiting > 0) {
-		alarm(0);
-		return (RP_TIMEOUT);
-	}
-
-	if (waiting > 0) {
+	if (timed_out != 0) {
 		tftp_log(LOG_ERR, "receive_packet: timeout");
 		alarm(0);
 		return (RP_TIMEOUT);
 	}
 
-	waiting++;
 	pfrom = (from == NULL) ? &from_local : from;
 	fromlen = sizeof(*pfrom);
 	n = recvfrom(peer, data, size, 0, (struct sockaddr *)pfrom, &fromlen);
@@ -432,8 +426,6 @@ receive_packet(int peer, char *data, int size, struct 
 		tftp_log(LOG_ERR, "receive_packet: timeout");
 		return (RP_TIMEOUT);
 	}
-
-	alarm(0);
 
 	if (n < 0) {
 		/* No idea what could have happened if it isn't a timeout */

Modified: head/libexec/tftpd/tftp-utils.c
==============================================================================
--- head/libexec/tftpd/tftp-utils.c	Sun Jul 22 14:11:52 2018	(r336604)
+++ head/libexec/tftpd/tftp-utils.c	Sun Jul 22 16:14:30 2018	(r336605)
@@ -270,11 +270,13 @@ char *
 rp_strerror(int error)
 {
 	static char s[100];
+	size_t space = sizeof(s);
 	int i = 0;
 
 	while (rp_errors[i].desc != NULL) {
 		if (rp_errors[i].error == error) {
-			strcpy(s, rp_errors[i].desc);
+			strlcpy(s, rp_errors[i].desc, space);
+			space -= strlen(rp_errors[i].desc);
 		}
 		i++;
 	}

Modified: head/libexec/tftpd/tftpd.c
==============================================================================
--- head/libexec/tftpd/tftpd.c	Sun Jul 22 14:11:52 2018	(r336604)
+++ head/libexec/tftpd/tftpd.c	Sun Jul 22 16:14:30 2018	(r336605)
@@ -374,7 +374,10 @@ main(int argc, char *argv[])
 			exit(1);
 		}
 		chdir("/");
-		setgroups(1, &nobody->pw_gid);
+		if (setgroups(1, &nobody->pw_gid) != 0) {
+			tftp_log(LOG_ERR, "setgroups failed");
+			exit(1);
+		}
 		if (setuid(nobody->pw_uid) != 0) {
 			tftp_log(LOG_ERR, "setuid failed");
 			exit(1);
@@ -522,7 +525,7 @@ tftp_wrq(int peer, char *recvbuffer, ssize_t size)
 	cp = parse_header(peer, recvbuffer, size, &filename, &mode);
 	size -= (cp - recvbuffer) + 1;
 
-	strcpy(fnbuf, filename);
+	strlcpy(fnbuf, filename, sizeof(fnbuf));
 	reduce_path(fnbuf);
 	filename = fnbuf;
 
@@ -567,7 +570,7 @@ tftp_rrq(int peer, char *recvbuffer, ssize_t size)
 	cp = parse_header(peer, recvbuffer, size, &filename, &mode);
 	size -= (cp - recvbuffer) + 1;
 
-	strcpy(fnbuf, filename);
+	strlcpy(fnbuf, filename, sizeof(fnbuf));
 	reduce_path(fnbuf);
 	filename = fnbuf;
 
@@ -804,6 +807,7 @@ tftp_xmitfile(int peer, const char *mode)
 	time_t now;
 	struct tftp_stats ts;
 
+	memset(&ts, 0, sizeof(ts));
 	now = time(NULL);
 	if (debug&DEBUG_SIMPLE)
 		tftp_log(LOG_DEBUG, "Transmitting file");