[OpenWrt-Devel] [PATCH v4][netifd] vlan device: Prevent interface name buffer overflow

[cshored at thecshore.com]

From: "Daniel F. Dickinson" <cshored at thecshore.com>

Avoid a buffer overflow because interface name grow
too long due to adding VLAN id or a prefix.  Log
and return an error result if the name does not
fit IFNAMSIZ (including terminating NUL).

Signed-off-by: Daniel F. Dickinson <cshored at thecshore.com>
---
Very lightly tested.  Logs and prevent creation of too long interface,
however the old interface (in case of network rename) still gets removed
which could cause lost of connectivity.  Not sure how to prevent that.

 device.c |  6 ++++++
 vlan.c   | 22 +++++++++++++++++-----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/device.c b/device.c
index a851037..5297693 100644
--- a/device.c
+++ b/device.c
@@ -664,6 +664,12 @@ int device_set_ifname(struct device *dev, const char *name)
 	if (!strcmp(dev->ifname, name))
 		return 0;
 
+	if (strnlen(name, IFNAMSIZ) >= IFNAMSIZ) {
+		netifd_log_message(L_WARNING, "Interface name '%s' too long\n",
+			name);
+		return ENAMETOOLONG;
+	}
+
 	if (dev->avl.key)
 		avl_delete(&devices, &dev->avl);
 
diff --git a/vlan.c b/vlan.c
index 067f624..f15d32f 100644
--- a/vlan.c
+++ b/vlan.c
@@ -61,13 +61,23 @@ static int vlan_set_device_state(struct device *dev, bool up)
 	return ret;
 }
 
-static void vlan_dev_set_name(struct vlan_device *vldev, struct device *dev)
+static int vlan_dev_set_name(struct vlan_device *vldev, struct device *dev)
 {
-	char name[IFNAMSIZ];
+	int ret = 0;
+
+	/* allow for adding dot + VLAN id */
+	char name[IFNAMSIZ + 6];
 
 	vldev->dev.hidden = dev->hidden;
-	snprintf(name, IFNAMSIZ, "%s.%d", dev->ifname, vldev->id);
-	device_set_ifname(&vldev->dev, name);
+	/* Truncate dev->ifname so that adding dot + VLAN id + '\0' doesn't exceed IFNAMSIZ */
+	snprintf(name, sizeof(name), "%.*s.%d", IFNAMSIZ, dev->ifname, vldev->id & 0xfff);
+	if (strnlen(name, IFNAMSIZ) >= IFNAMSIZ) {
+		netifd_log_message(L_WARNING, "VLAN interface name '%s' too long", name);
+		ret = ENAMETOOLONG;
+	} else {
+		device_set_ifname(&vldev->dev, name);
+	}
+	return ret;
 }
 
 static void vlan_dev_cb(struct device_user *dep, enum device_event ev)
@@ -129,7 +139,9 @@ static struct device *get_vlan_device(struct device *dev, int id, bool create)
 
 	device_init(&vldev->dev, &vlan_type, NULL);
 
-	vlan_dev_set_name(vldev, dev);
+	if (vlan_dev_set_name(vldev, dev))
+		return NULL;
+
 	vldev->dev.default_config = true;
 
 	vldev->set_state = vldev->dev.set_state;
-- 
2.14.1
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel