diff mbox series

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

Message ID 20180130193335.22225-1-cshored@thecshore.com
State Superseded
Headers show
Series [OpenWrt-Devel,v2] vlan: Buffer overlow in snprintf for vlans | expand

Commit Message

Daniel Dickinson Jan. 30, 2018, 7:33 p.m. UTC 
From: "Daniel F. Dickinson" <cshored@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@thecshore.com>
---
 vlan.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
diff mbox series

Patch

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);
 }