[OpenWrt-Devel] [PATCH v2] vlan: Buffer overlow in snprintf for vlans

[cshored at thecshore.com]

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

Ok, found a way to test the long end of the range, and fixed
the off by 2 error in the last patch.  Stil more informational,
but I hope you find it useful.

Buffer overlflow condition can occur because vlan
device name is constructed from device name (size IFNAMSIZ)
plus the ASCII decimal representation of the vlan id plus
a dot, but the target can only be IFNAMSIZ.  We fix this
by using fields widths (and make sure we don't truncate
more of the orogin device name than we must).

Signed-off-by: Daniel F. Dickinson <cshored at thecshore.com>
---
 vlan.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/vlan.c b/vlan.c
index 067f624..eb20b13 100644
--- a/vlan.c
+++ b/vlan.c
@@ -64,9 +64,19 @@ static int vlan_set_device_state(struct device *dev, bool up)
 static void vlan_dev_set_name(struct vlan_device *vldev, struct device *dev)
 {
 	char name[IFNAMSIZ];
+	char devnum[5];
+	int i, j = 0;
 
 	vldev->dev.hidden = dev->hidden;
-	snprintf(name, IFNAMSIZ, "%s.%d", dev->ifname, vldev->id);
+	snprintf(devnum, 5, "%d", vldev->id);
+	i = strnlen(devnum, 4);
+	/* Subtract the dot and terminating null */
+	j = IFNAMSIZ - i - 3;
+	/* Brute force the null and length and 0-index math */
+	name[0] = 0;
+	strncat(name, dev->ifname, j);
+	strncat(name, ".", 1);
+	strncat(name, devnum, i);
 	device_set_ifname(&vldev->dev, name);
 }
 
-- 
2.11.0
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel