summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHauke Mehrtens2026-05-23 12:34:33 +0000
committerHauke Mehrtens2026-06-18 00:35:09 +0000
commit2db29c4eee329dd2d75519215bfda89df69ffd8e (patch)
tree550731d76e1f2b6411e031663ec01b4e5c240cdc
parent0721ab6f06d68f5484c0d8c6f81a386d1fce1089 (diff)
downloadodhcpd-2db29c4eee329dd2d75519215bfda89df69ffd8e.tar.gz
dhcpv4: copy ifname into arpreq without reading past the source
dhcpv4_set_dest_addr() filled arp.arp_dev (a fixed 16-byte field) via memcpy(..., iface->ifname, sizeof(arp.arp_dev)). iface->ifname is a strdup'd string whose allocation is just strlen(ifname)+1 bytes — for typical names like "eth0" or "br-lan" that is 5–7 bytes, so the memcpy reads ~10 bytes past the heap allocation. The bytes happen to be inside the same malloc chunk on glibc so it has never blown up in practice, but it is undefined behaviour and would trip valgrind / ASan. Switch to the strncpy form already used for ifr_name elsewhere; the struct is zero-initialised so the trailing byte stays NUL. Assisted-by: Claude:claude-opus-4-7 Link: https://github.com/openwrt/odhcpd/pull/401 Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> (cherry picked from commit 7c834a54ae6da732edaf2e8021fa3681e69951a3)
-rw-r--r--src/dhcpv4.c6
1 files changed, 5 insertions, 1 deletions
diff --git a/src/dhcpv4.c b/src/dhcpv4.c
index 0ef01c3..e58de40 100644
--- a/src/dhcpv4.c
+++ b/src/dhcpv4.c
@@ -753,7 +753,11 @@ static void dhcpv4_set_dest_addr(const struct interface *iface,
memcpy(arp.arp_ha.sa_data, req->chaddr, 6);
memcpy(&arp.arp_pa, dest, sizeof(arp.arp_pa));
- memcpy(arp.arp_dev, iface->ifname, sizeof(arp.arp_dev));
+ /* arp_dev is a 16-byte fixed buffer; strdup'd ifname is
+ * typically shorter than that, so memcpy()ing the full
+ * field length reads past the allocation. Match the
+ * strncpy pattern used elsewhere for ifr_name. */
+ strncpy(arp.arp_dev, iface->ifname, sizeof(arp.arp_dev) - 1);
if (ioctl(iface->dhcpv4_event.uloop.fd, SIOCSARP, &arp) < 0)
error("ioctl(SIOCSARP): %m");