git: c15290fb9d8f - main - tftpd: Code cleanup.

The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=c15290fb9d8fdf4b11b9c6e7406b67c73a98402d

commit c15290fb9d8fdf4b11b9c6e7406b67c73a98402d
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2024-07-05 22:05:49 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2024-07-05 22:05:49 +0000

    tftpd: Code cleanup.
    
    MFC after:      3 days
    Sponsored by:   Klara, Inc.
    Reviewed by:    markj
    Differential Revision:  https://reviews.freebsd.org/D45871
---
 libexec/tftpd/tftpd.c | 91 +++++++++++++++++++++++----------------------------
 1 file changed, 41 insertions(+), 50 deletions(-)

diff --git a/libexec/tftpd/tftpd.c b/libexec/tftpd/tftpd.c
index 3f67ad2920cf..a51fb4742985 100644
--- a/libexec/tftpd/tftpd.c
+++ b/libexec/tftpd/tftpd.c
@@ -680,28 +680,27 @@ find_next_name(char *filename, int *fd)
 int
 validate_access(int peer, char **filep, int mode)
 {
-	struct stat stbuf;
-	int	fd;
-	int	error;
-	struct dirlist *dirp;
 	static char pathname[MAXPATHLEN];
+	struct stat sb;
+	struct dirlist *dirp;
 	char *filename = *filep;
+	int	err, fd;
 
 	/*
 	 * Prevent tricksters from getting around the directory restrictions
 	 */
-	if (strstr(filename, "/../"))
+	if (strncmp(filename, "../", 3) == 0 ||
+	    strstr(filename, "/../") != NULL)
 		return (EACCESS);
 
 	if (*filename == '/') {
 		/*
-		 * Allow the request if it's in one of the approved locations.
-		 * Special case: check the null prefix ("/") by looking
-		 * for length = 1 and relying on the arg. processing that
-		 * it's a /.
+		 * Absolute file name: allow the request if it's in one of the
+		 * approved locations.
 		 */
 		for (dirp = dirs; dirp->name != NULL; dirp++) {
 			if (dirp->len == 1)
+				/* Only "/" can have len 1 */
 				break;
 			if (strncmp(filename, dirp->name, dirp->len) == 0 &&
 			    filename[dirp->len] == '/')
@@ -710,30 +709,20 @@ validate_access(int peer, char **filep, int mode)
 		/* If directory list is empty, allow access to any file */
 		if (dirp->name == NULL && dirp != dirs)
 			return (EACCESS);
-		if (stat(filename, &stbuf) < 0)
+		if (stat(filename, &sb) != 0)
 			return (errno == ENOENT ? ENOTFOUND : EACCESS);
-		if ((stbuf.st_mode & S_IFMT) != S_IFREG)
+		if (!S_ISREG(sb.st_mode))
 			return (ENOTFOUND);
 		if (mode == RRQ) {
-			if ((stbuf.st_mode & S_IROTH) == 0)
+			if ((sb.st_mode & S_IROTH) == 0)
 				return (EACCESS);
 		} else {
-			if (check_woth && ((stbuf.st_mode & S_IWOTH) == 0))
+			if (check_woth && (sb.st_mode & S_IWOTH) == 0)
 				return (EACCESS);
 		}
 	} else {
-		int err;
-
 		/*
 		 * Relative file name: search the approved locations for it.
-		 * Don't allow write requests that avoid directory
-		 * restrictions.
-		 */
-
-		if (!strncmp(filename, "../", 3))
-			return (EACCESS);
-
-		/*
 		 * If the file exists in one of the directories and isn't
 		 * readable, continue looking. However, change the error code
 		 * to give an indication that the file exists.
@@ -741,18 +730,20 @@ validate_access(int peer, char **filep, int mode)
 		err = ENOTFOUND;
 		for (dirp = dirs; dirp->name != NULL; dirp++) {
 			snprintf(pathname, sizeof(pathname), "%s/%s",
-				dirp->name, filename);
-			if (stat(pathname, &stbuf) == 0 &&
-			    (stbuf.st_mode & S_IFMT) == S_IFREG) {
-				if (mode == RRQ) {
-					if ((stbuf.st_mode & S_IROTH) != 0)
-						break;
-				} else {
-					if (!check_woth || ((stbuf.st_mode & S_IWOTH) != 0))
-						break;
-				}
-				err = EACCESS;
+			    dirp->name, filename);
+			if (stat(pathname, &sb) != 0)
+				continue;
+			if (!S_ISREG(sb.st_mode))
+				continue;
+			err = EACCESS;
+			if (mode == RRQ) {
+				if ((sb.st_mode & S_IROTH) == 0)
+					continue;
+			} else {
+				if (check_woth && (sb.st_mode & S_IWOTH) == 0)
+					continue;
 			}
+			break;
 		}
 		if (dirp->name != NULL)
 			*filep = filename = pathname;
@@ -766,27 +757,27 @@ validate_access(int peer, char **filep, int mode)
 	 * This option is handled here because it (might) require(s) the
 	 * size of the file.
 	 */
-	option_tsize(peer, NULL, mode, &stbuf);
+	option_tsize(peer, NULL, mode, &sb);
 
-	if (mode == RRQ)
+	if (mode == RRQ) {
 		fd = open(filename, O_RDONLY);
-	else {
-		if (create_new) {
-			if (increase_name) {
-				error = find_next_name(filename, &fd);
-				if (error > 0)
-					return (error + 100);
-			} else
-				fd = open(filename,
-				    O_WRONLY | O_TRUNC | O_CREAT,
-				    S_IRUSR | S_IWUSR | S_IRGRP |
-				    S_IWGRP | S_IROTH | S_IWOTH );
-		} else
-			fd = open(filename, O_WRONLY | O_TRUNC);
+	} else if (create_new) {
+		if (increase_name) {
+			err = find_next_name(filename, &fd);
+			if (err > 0)
+				return (err + 100);
+		} else {
+			fd = open(filename,
+			    O_WRONLY | O_TRUNC | O_CREAT,
+			    S_IRUSR | S_IWUSR | S_IRGRP |
+			    S_IWGRP | S_IROTH | S_IWOTH );
+		}
+	} else {
+		fd = open(filename, O_WRONLY | O_TRUNC);
 	}
 	if (fd < 0)
 		return (errno + 100);
-	file = fdopen(fd, (mode == RRQ)? "r":"w");
+	file = fdopen(fd, mode == RRQ ? "r" : "w");
 	if (file == NULL) {
 		close(fd);
 		return (errno + 100);