net: ip: account for the size in the inet_ntop code path

The code was writing to the dst without a verification check on
size which is not appropriate. The guard on the arguements should
be enforced and so a stack manipulation followed by a strcpy is
slight less efficient but at the benefit of being memory safe from
the args being passed in.

Signed-off-by: Charles Hardin <[email protected]>

Author: ckhardin

subsys/net/ip/utils.c

@@ -168,11 +168,15 @@ char *z_impl_net_addr_ntop(sa_family_t family, const void *src,
 	int pos = -1;
 	char delim = ':';
 	uint8_t zeros[8] = { 0 };
-	char *ptr = dst;
 	int len = -1;
 	uint16_t value;
 	bool needcolon = false;
 	bool mapped = false;
+	char *sptr, *ptr;
+	union {
+		char instr[INET_ADDRSTRLEN];
+		char in6str[INET6_ADDRSTRLEN];
+	} tmp;
 
 	if (family == AF_INET6) {
 		net_ipv6_addr_copy_raw(addr6.s6_addr, src);
@@ -204,14 +208,17 @@ char *z_impl_net_addr_ntop(sa_family_t family, const void *src,
 			pos = -1;
 		}
 
+		sptr = &tmp.in6str[0];
 	} else if (family == AF_INET) {
 		net_ipv4_addr_copy_raw(addr.s4_addr, src);
 		len = 4;
 		delim = '.';
+		sptr = &tmp.instr[0];
 	} else {
 		return NULL;
 	}
 
+	ptr = sptr;
 print_mapped:
 	for (i = 0; i < len; i++) {
 		/* IPv4 address a.b.c.d */
@@ -281,16 +288,18 @@ char *z_impl_net_addr_ntop(sa_family_t family, const void *src,
 		needcolon = true;
 	}
 
-	if (!(ptr - dst)) {
-		return NULL;
-	}
-
 	if (family == AF_INET) {
+		/* delim was written as last character - overwrite with nil */
 		*(ptr - 1) = '\0';
 	} else {
-		*ptr = '\0';
+		/* nil terminate and increment to compute the size */
+		*ptr++ = '\0';
 	}
 
+	if ((size_t)(ptr - sptr) > size) {
+		return NULL;
+	}
+	strcpy(dst, sptr);
 	return dst;
 }