diff mbox series

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

Message ID 20180225232227.8500-1-cshored@thecshore.com
State Superseded
Delegated to: John Crispin
Headers show
Series [OpenWrt-Devel,v4,netifd] vlan device: Prevent interface name buffer overflow | expand

Commit Message

Daniel Dickinson Feb. 25, 2018, 11:22 p.m. UTC 
From: "Daniel F. Dickinson" <cshored@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@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 mbox series

Patch

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;