[OpenWrt-Devel] [PATCH] [netifd] vlan: Array out of bounds in snprintf for vlans

[Paul Oranje]

Why use a hard coded value 4 in "snprintf(devnum, 4, "%d", vldev->id);" ?
Paul

> Op 30 jan. 2018, om 19:16 heeft cshored at thecshore.com het volgende geschreven:
> 
> From: "Daniel F. Dickinson" <cshored at thecshore.com>
> 
> Detected during a side project.  Not a brilliant fix, but it
> gets the job done for now.  *very* lightly tested, more
> for your information than anything else.
> 
> Array out-of-bounds 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 | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/vlan.c b/vlan.c
> index 067f624..44852f4 100644
> --- a/vlan.c
> +++ b/vlan.c
> @@ -63,10 +63,17 @@ 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 name[IFNAMSIZ + 1];
> +	char devnum[5];
> +	int i, j = 0;
> 
> 	vldev->dev.hidden = dev->hidden;
> -	snprintf(name, IFNAMSIZ, "%s.%d", dev->ifname, vldev->id);
> +	snprintf(devnum, 4, "%d", vldev->id);
> +	i = strnlen(devnum, 4);
> +	j = IFNAMSIZ - i;
> +	strncpy(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
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel