[PATCH]ethtool.c:fix buffer overflow when devname is too long

[PATCH]ethtool.c:fix buffer overflow when devname is too long

[wangzyu]

As the length of field ifr_name of struct ifreq is IFNAMSIZ(16) in header file /usr/include/net/if.h. 
It will result in buffer overflow when devname is too long. Modified strcpy to strncpy for only 
copying IFNAMSIZ bytes into struct ifreq. Also, by adding a section into parse_cmdline to detect 
if the length of devname is invalid.

Signed-off-by: Zhao Yu Wang <wangzyu@cn.ibm.com>
--- ethtool-4.orig/ethtool.c	2006-07-18 21:21:38.000000000 -0500
+++ ethtool-4/ethtool.c	2006-08-27 22:32:12.000000000 -0500
@@ -626,6 +626,9 @@ static void parse_cmdline(int argc, char
 
 	if (devname == NULL) {
 		show_usage(1);
+	} else if (strlen(devname) > IFNAMSIZ) {
+		fprintf(stderr, "Device name is too long. Should be less than %d!\n", IFNAMSIZ);
+		show_usage(1);
 	}
 }
 
@@ -1139,7 +1142,7 @@ static int doit(void)
 
 	/* Setup our control structures. */
 	memset(&ifr, 0, sizeof(ifr));
-	strcpy(ifr.ifr_name, devname);
+	strncpy(ifr.ifr_name, devname, IFNAMSIZ);
 
 	/* Open control socket. */
 	fd = socket(AF_INET, SOCK_DGRAM, 0);

Re: [PATCH]ethtool.c:fix buffer overflow when devname is too long

[Jeff Garzik]

wangzyu@cn.ibm.com wrote:
> As the length of field ifr_name of struct ifreq is IFNAMSIZ(16) in header file /usr/include/net/if.h. 
> It will result in buffer overflow when devname is too long. Modified strcpy to strncpy for only 
> copying IFNAMSIZ bytes into struct ifreq. Also, by adding a section into parse_cmdline to detect 
> if the length of devname is invalid.
> 
> Signed-off-by: Zhao Yu Wang <wangzyu@cn.ibm.com>

There's already a patch checked into ethtool.git for this...

	Jeff