Subject: if_clone_lookup bogus?
To: None <tech-net@netbsd.org>
From: Quentin Garnier <netbsd@quatriemek.com>
List: tech-net
Date: 08/14/2003 01:40:48
Maybe I'm mistaken, but if_clone_lookup in sys/net/if.c looks bogus to me.

Imagine the case where both ppp(4) and pppoe(4) are clonable and compiled,
and "ppp" appears before "pppoe" in the cloners list.

When requesting creation of a pppoe interface, if_clone_lookup will
mistakenly match "ppp" and then fails because it is not followed by a
number.

I propose the following patch. It makes the thing (slightly) less
obfuscated and make some boundary check on the name parameter.

Index: if.c
===================================================================
RCS file: /cvsroot/src/sys/net/if.c,v
retrieving revision 1.127
diff -u -r1.127 if.c
--- if.c	2003/08/09 18:13:03	1.127
+++ if.c	2003/08/13 23:29:10
@@ -795,24 +795,27 @@
 	int *unitp;
 {
 	struct if_clone *ifc;
-	const char *cp;
-	size_t i;
+	const char *cp = name;
+	size_t i, j;
 
-	for (ifc = LIST_FIRST(&if_cloners); ifc != NULL;) {
-		for (cp = name, i = 0; i < ifc->ifc_namelen; i++, cp++) {
-			if (ifc->ifc_name[i] != *cp)
-				goto next_ifc;
-		}
-		goto found_name;
- next_ifc:
-		ifc = LIST_NEXT(ifc, ifc_list);
-	}
+	/* separate interface name from unit */
+	for (j = 0;
+	    j < IFNAMSIZ && *cp && *cp < '0' && *cp > '9';
+	    j++, cp++)
+		continue;
 
-	/* No match. */
-	return (NULL);
+	if (!j || j == IFNAMSIZ || !*cp)
+	    return (NULL); /* No name or unit number */
 
- found_name:
-	for (i = 0; *cp != '\0'; cp++) {
+	LIST_FOREACH(ifc, &if_cloners, ifc_list)
+		if (strlen(ifc->ifc_name) == j &&
+		    !strncmp(name, ifc->ifc_name, j))
+			break;
+
+	if (ifc == NULL)
+		return (NULL);
+
+	for (i = 0; j < IFNAMSIZ && *cp != '\0'; cp++, j++) {
 		if (*cp < '0' || *cp > '9') {
 			/* Bogus unit number. */
 			return (NULL);

-- 
Quentin Garnier - cube@cubidou.net
"Feels like I'm fiddling while Rome is burning down.
Should I lay my fiddle down and take a rifle from the ground ?"
Leigh Nash/Sixpence None The Richer, Paralyzed, Divine Discontents, 2002.