The arpreq.arp_dev is a limited buffer (16 bytes). Avoid that more bytes from the interface name are copied into this buffer by switching from strcpy to strncpy.
Fixes: c7da798113a2 ("alfred: IPv4 multicast distribution support.") Signed-off-by: Sven Eckelmann sven@narfation.org --- util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/util.c b/util.c index 84ab3af..ed83895 100644 --- a/util.c +++ b/util.c @@ -92,7 +92,9 @@ int ipv4_arp_request(struct interface *interface, const alfred_addr *addr, sin->sin_family = AF_INET; sin->sin_addr.s_addr = addr->ipv4.s_addr;
- strcpy(arpreq.arp_dev, interface->interface); + strncpy(arpreq.arp_dev, interface->interface, sizeof(arpreq.arp_dev)); + arpreq.arp_dev[sizeof(arpreq.arp_dev) - 1] = '\0'; + if (ioctl(interface->netsock, SIOCGARP, &arpreq) < 0) return -1;
On Fri, Jan 27, 2017 at 03:10:44PM +0100, Sven Eckelmann wrote:
The arpreq.arp_dev is a limited buffer (16 bytes). Avoid that more bytes from the interface name are copied into this buffer by switching from strcpy to strncpy.
Fixes: c7da798113a2 ("alfred: IPv4 multicast distribution support.") Signed-off-by: Sven Eckelmann sven@narfation.org
util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/util.c b/util.c index 84ab3af..ed83895 100644 --- a/util.c +++ b/util.c @@ -92,7 +92,9 @@ int ipv4_arp_request(struct interface *interface, const alfred_addr *addr, sin->sin_family = AF_INET; sin->sin_addr.s_addr = addr->ipv4.s_addr;
- strcpy(arpreq.arp_dev, interface->interface);
- strncpy(arpreq.arp_dev, interface->interface, sizeof(arpreq.arp_dev));
arpreq is already set to 0 few lines above. why not simpling "sizeof(arpreq.arp_dev) - 1" as last argument for the strncpy() and avoid the line below?
Or is this required for consistency with the rest of the code?
Cheers,
On Freitag, 27. Januar 2017 22:34:22 CET Antonio Quartulli wrote: [...]
arpreq is already set to 0 few lines above. why not simpling "sizeof(arpreq.arp_dev) - 1" as last argument for the strncpy() and avoid the line below?
Or is this required for consistency with the rest of the code?
It is done for consistency.
Kind regards, Sven
On Friday, January 27, 2017 3:10:44 PM CET Sven Eckelmann wrote:
The arpreq.arp_dev is a limited buffer (16 bytes). Avoid that more bytes from the interface name are copied into this buffer by switching from strcpy to strncpy.
Fixes: c7da798113a2 ("alfred: IPv4 multicast distribution support.") Signed-off-by: Sven Eckelmann sven@narfation.org
Committed in revision 26ef92b.
Thank you, Simon
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Simon Wunderlich -
Sven Eckelmann