git: b1ece85741db - stable/13 - dhclient: Improve server and filename validation

The branch stable/13 has been updated by des:

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

commit b1ece85741db0db60ce68daca564f03dc711fe30
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2026-04-30 16:45:35 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2026-04-30 21:07:18 +0000

    dhclient: Improve server and filename validation
    
    * Don't iterate over each string three times; once is enough.
    
    * Reject control characters (anything below space) in addition to the
      double quote and backslash.
    
    * If an unsafe character is encountered, discard the string instead of
      rejecting the entire lease.
    
    * If backslashes are encountered in the file name option, convert them
      to forward slashes instead of rejecting the option.
    
    * Tweak the warning messages a bit.  Looking through the rest of the
      code, it seems to me that notes generally end with a period while
      warnings generally don't.
    
    Fixes:          8008e4b88daf ("dhclient: Check for unexpected characters in some DHCP server options")
    PR:             294886
    MFC after:      1 week
    Reviewed by:    brooks, markj
    Differential Revision:  https://reviews.freebsd.org/D56740
    
    (cherry picked from commit 873a195ba63575e46686cfd6ea9670a0ca340fa0)
---
 sbin/dhclient/dhclient.c | 75 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 21 deletions(-)

diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index 6a2683fec92a..1e72c304495c 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -1161,7 +1161,7 @@ packet_to_lease(struct packet *packet)
 	lease = malloc(sizeof(struct client_lease));
 
 	if (!lease) {
-		warning("dhcpoffer: no memory to record lease.");
+		warning("dhcpoffer: no memory to record lease");
 		return (NULL);
 	}
 
@@ -1202,7 +1202,7 @@ packet_to_lease(struct packet *packet)
 
 	/* If the server name was filled out, copy it.
 	   Do not attempt to validate the server name as a host name.
-	   RFC 2131 merely states that sname is NUL-terminated (which do
+	   RFC 2131 merely states that sname is NUL-terminated (which we
 	   do not assume) and that it is the server's host name.  Since
 	   the ISC client and server allow arbitrary characters, we do
 	   as well. */
@@ -1210,39 +1210,72 @@ packet_to_lease(struct packet *packet)
 	    !(packet->options[DHO_DHCP_OPTION_OVERLOAD].data[0] & 2)) &&
 	    packet->raw->sname[0]) {
 		lease->server_name = malloc(DHCP_SNAME_LEN + 1);
-		if (!lease->server_name) {
-			warning("dhcpoffer: no memory for server name.");
+		if (lease->server_name == NULL) {
+			warning("dhcpoffer: no memory for server name");
 			free_client_lease(lease);
 			return (NULL);
 		}
-		memcpy(lease->server_name, packet->raw->sname, DHCP_SNAME_LEN);
-		lease->server_name[DHCP_SNAME_LEN]='\0';
-		if (strchr(lease->server_name, '"') != NULL ||
-		    strchr(lease->server_name, '\\') != NULL) {
-			warning("dhcpoffer: server name contains invalid characters.");
-			free_client_lease(lease);
-			return (NULL);
+		for (i = 0; i < DHCP_SNAME_LEN; i++) {
+			if (packet->raw->sname[i] == '\0') {
+				break;
+			}
+			if (packet->raw->sname[i] < ' ' ||
+			    packet->raw->sname[i] == '"' ||
+			    packet->raw->sname[i] == '\\') {
+				warning("dhcpoffer: server name contains "
+				    "unsafe characters");
+				free(lease->server_name);
+				lease->server_name = NULL;
+				break;
+			}
+			lease->server_name[i] = packet->raw->sname[i];
+		}
+		/* Terminate and zero-pad */
+		if (lease->server_name != NULL) {
+			while (i < DHCP_SNAME_LEN + 1) {
+				lease->server_name[i++] = '\0';
+			}
 		}
 	}
 
-	/* Ditto for the filename. */
+	/* Ditto for the file name. */
 	if ((!packet->options[DHO_DHCP_OPTION_OVERLOAD].len ||
 	    !(packet->options[DHO_DHCP_OPTION_OVERLOAD].data[0] & 1)) &&
 	    packet->raw->file[0]) {
 		/* Don't count on the NUL terminator. */
 		lease->filename = malloc(DHCP_FILE_LEN + 1);
-		if (!lease->filename) {
-			warning("dhcpoffer: no memory for filename.");
+		if (lease->filename == NULL) {
+			warning("dhcpoffer: no memory for file name");
 			free_client_lease(lease);
 			return (NULL);
 		}
-		memcpy(lease->filename, packet->raw->file, DHCP_FILE_LEN);
-		lease->filename[DHCP_FILE_LEN]='\0';
-		if (strchr(lease->filename, '"') != NULL ||
-		    strchr(lease->filename, '\\') != NULL) {
-			warning("dhcpoffer: filename contains invalid characters.");
-			free_client_lease(lease);
-			return (NULL);
+		for (i = 0; i < DHCP_FILE_LEN; i++) {
+			if (packet->raw->file[i] == '\0') {
+				break;
+			}
+			if (packet->raw->file[i] < ' ' ||
+			    packet->raw->file[i] == '"') {
+				warning("dhcpoffer: file name contains "
+				    "unsafe characters");
+				free(lease->filename);
+				lease->filename = NULL;
+				break;
+			}
+			if (packet->raw->file[i] == '\\') {
+				/*
+				 * This is common in Windows-centric
+				 * environments.  Instead of rejecting,
+				 * silently convert to forward slash.
+				 */
+				packet->raw->file[i] = '/';
+			}
+			lease->filename[i] = packet->raw->file[i];
+		}
+		/* Terminate and zero-pad */
+		if (lease->filename != NULL) {
+			while (i < DHCP_FILE_LEN + 1) {
+				lease->filename[i++] = '\0';
+			}
 		}
 	}
 	return lease;