From 47b45b2967c931fed3c89a2e6a8df9f9183a5789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Wed, 15 Aug 2018 18:17:00 +0200 Subject: [PATCH] Fix lengths of interface names Use helper function similar to copy correctly limited names into buffers. --- src/bpf.c | 2 +- src/dhcp-common.c | 5 ++++- src/dhcp.c | 9 ++++----- src/dnsmasq.h | 1 + src/log.c | 2 +- src/netlink.c | 5 +++++ src/network.c | 12 ++++++------ src/option.c | 9 ++++----- src/rfc2131.c | 10 +++++----- src/tftp.c | 2 +- src/util.c | 13 ++++++++++++- 11 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/bpf.c b/src/bpf.c index 49a11bf..ff66d6d 100644 --- a/src/bpf.c +++ b/src/bpf.c @@ -169,7 +169,7 @@ int iface_enumerate(int family, void *parm, int (*callback)()) struct in6_ifreq ifr6; memset(&ifr6, 0, sizeof(ifr6)); - strncpy(ifr6.ifr_name, addrs->ifa_name, sizeof(ifr6.ifr_name)); + safe_strncpy(ifr6.ifr_name, addrs->ifa_name, sizeof(ifr6.ifr_name)); ifr6.ifr_addr = *((struct sockaddr_in6 *) addrs->ifa_addr); if (fd != -1 && ioctl(fd, SIOCGIFAFLAG_IN6, &ifr6) != -1) diff --git a/src/dhcp-common.c b/src/dhcp-common.c index 8ff0f0d..78c1d9b 100644 --- a/src/dhcp-common.c +++ b/src/dhcp-common.c @@ -485,8 +485,11 @@ char *whichdevice(void) void bindtodevice(char *device, int fd) { + size_t len = strlen(device)+1; + if (len > IFNAMSIZ) + len = IFNAMSIZ; /* only allowed by root. */ - if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, device, IFNAMSIZ) == -1 && + if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, device, len) == -1 && errno != EPERM) die(_("failed to set SO_BINDTODEVICE on DHCP socket: %s"), NULL, EC_BADNET); } diff --git a/src/dhcp.c b/src/dhcp.c index 4d29525..f8d323b 100644 --- a/src/dhcp.c +++ b/src/dhcp.c @@ -232,7 +232,7 @@ void dhcp_packet(time_t now, int pxe_fd) #ifdef HAVE_LINUX_NETWORK /* ARP fiddling uses original interface even if we pretend to use a different one. */ - strncpy(arp_req.arp_dev, ifr.ifr_name, 16); + safe_strncpy(arp_req.arp_dev, ifr.ifr_name, sizeof(arp_req.arp_dev)); #endif /* If the interface on which the DHCP request was received is an @@ -255,7 +255,7 @@ void dhcp_packet(time_t now, int pxe_fd) } else { - strncpy(ifr.ifr_name, bridge->iface, IF_NAMESIZE); + safe_strncpy(ifr.ifr_name, bridge->iface, sizeof(ifr.ifr_name)); break; } } @@ -279,7 +279,7 @@ void dhcp_packet(time_t now, int pxe_fd) is_relay_reply = 1; iov.iov_len = sz; #ifdef HAVE_LINUX_NETWORK - strncpy(arp_req.arp_dev, ifr.ifr_name, 16); + safe_strncpy(arp_req.arp_dev, ifr.ifr_name, sizeof(arp_req.arp_dev)); #endif } else @@ -988,8 +988,7 @@ char *host_from_dns(struct in_addr addr) if (!legal_hostname(hostname)) return NULL; - strncpy(daemon->dhcp_buff, hostname, 256); - daemon->dhcp_buff[255] = 0; + safe_strncpy(daemon->dhcp_buff, hostname, 256); strip_hostname(daemon->dhcp_buff); return daemon->dhcp_buff; diff --git a/src/dnsmasq.h b/src/dnsmasq.h index a416b60..e85ec2c 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -1252,6 +1252,7 @@ int legal_hostname(char *name); char *canonicalise(char *in, int *nomem); unsigned char *do_rfc1035_name(unsigned char *p, char *sval, char *limit); void *safe_malloc(size_t size); +void safe_strncpy(char *dest, const char *src, size_t size); void safe_pipe(int *fd, int read_noblock); void *whine_malloc(size_t size); int sa_len(union mysockaddr *addr); diff --git a/src/log.c b/src/log.c index dae8a75..d0d4780 100644 --- a/src/log.c +++ b/src/log.c @@ -232,7 +232,7 @@ static void log_write(void) logaddr.sun_len = sizeof(logaddr) - sizeof(logaddr.sun_path) + strlen(_PATH_LOG) + 1; #endif logaddr.sun_family = AF_UNIX; - strncpy(logaddr.sun_path, _PATH_LOG, sizeof(logaddr.sun_path)); + safe_strncpy(logaddr.sun_path, _PATH_LOG, sizeof(logaddr.sun_path)); /* Got connection back? try again. */ if (connect(log_fd, (struct sockaddr *)&logaddr, sizeof(logaddr)) != -1) diff --git a/src/netlink.c b/src/netlink.c index 05153f5..849c2ff 100644 --- a/src/netlink.c +++ b/src/netlink.c @@ -149,10 +149,15 @@ int iface_enumerate(int family, void *parm, int (*callback)()) struct rtgenmsg g; } req; + memset(&req, 0, sizeof(req)); + memset(&addr, 0, sizeof(addr)); + addr.nl_family = AF_NETLINK; +#if 0 addr.nl_pad = 0; addr.nl_groups = 0; addr.nl_pid = 0; /* address to kernel */ +#endif again: if (family == AF_UNSPEC) diff --git a/src/network.c b/src/network.c index b405458..8ae7a70 100644 --- a/src/network.c +++ b/src/network.c @@ -29,7 +29,7 @@ int indextoname(int fd, int index, char *name) if (ioctl(fd, SIOCGIFNAME, &ifr) == -1) return 0; - strncpy(name, ifr.ifr_name, IF_NAMESIZE); + safe_strncpy(name, ifr.ifr_name, IF_NAMESIZE); return 1; } @@ -82,12 +82,12 @@ int indextoname(int fd, int index, char *name) for (i = lifc.lifc_len / sizeof(struct lifreq); i; i--, lifrp++) { struct lifreq lifr; - strncpy(lifr.lifr_name, lifrp->lifr_name, IF_NAMESIZE); + safe_strncpy(lifr.lifr_name, lifrp->lifr_name, IF_NAMESIZE); if (ioctl(fd, SIOCGLIFINDEX, &lifr) < 0) return 0; if (lifr.lifr_index == index) { - strncpy(name, lifr.lifr_name, IF_NAMESIZE); + safe_strncpy(name, lifr.lifr_name, IF_NAMESIZE); return 1; } } @@ -188,7 +188,7 @@ int loopback_exception(int fd, int family, struct all_addr *addr, char *name) struct ifreq ifr; struct irec *iface; - strncpy(ifr.ifr_name, name, IF_NAMESIZE); + safe_strncpy(ifr.ifr_name, name, IF_NAMESIZE); if (ioctl(fd, SIOCGIFFLAGS, &ifr) != -1 && ifr.ifr_flags & IFF_LOOPBACK) { @@ -1286,7 +1286,7 @@ static struct serverfd *allocate_sfd(union mysockaddr *addr, char *intname) return NULL; } - strcpy(sfd->interface, intname); + safe_strncpy(sfd->interface, intname, sizeof(sfd->interface)); sfd->source_addr = *addr; sfd->next = daemon->sfds; sfd->ifindex = ifindex; @@ -1458,7 +1458,7 @@ void add_update_server(int flags, serv->flags |= SERV_HAS_DOMAIN; if (interface) - strcpy(serv->interface, interface); + safe_strncpy(serv->interface, interface, sizeof(serv->interface)); if (addr) serv->addr = *addr; if (source_addr) diff --git a/src/option.c b/src/option.c index 6ffd5d7..7ccbdea 100644 --- a/src/option.c +++ b/src/option.c @@ -810,7 +810,7 @@ char *parse_server(char *arg, union mysockaddr *addr, union mysockaddr *source_a if (interface_opt) { #if defined(SO_BINDTODEVICE) - strncpy(interface, interface_opt, IF_NAMESIZE - 1); + safe_strncpy(interface, interface_opt, IF_NAMESIZE); #else return _("interface binding not supported"); #endif @@ -839,7 +839,7 @@ char *parse_server(char *arg, union mysockaddr *addr, union mysockaddr *source_a return _("interface can only be specified once"); source_addr->in.sin_addr.s_addr = INADDR_ANY; - strncpy(interface, source, IF_NAMESIZE - 1); + safe_strncpy(interface, source, IF_NAMESIZE); #else return _("interface binding not supported"); #endif @@ -874,7 +874,7 @@ char *parse_server(char *arg, union mysockaddr *addr, union mysockaddr *source_a return _("interface can only be specified once"); source_addr->in6.sin6_addr = in6addr_any; - strncpy(interface, source, IF_NAMESIZE - 1); + safe_strncpy(interface, source, IF_NAMESIZE); #else return _("interface binding not supported"); #endif @@ -4801,8 +4801,7 @@ void read_opts(int argc, char **argv, char *compile_opts) argbuf_size = strlen(optarg) + 1; argbuf = opt_malloc(argbuf_size); } - strncpy(argbuf, optarg, argbuf_size); - argbuf[argbuf_size-1] = 0; + safe_strncpy(argbuf, optarg, argbuf_size); arg = argbuf; } else diff --git a/src/rfc2131.c b/src/rfc2131.c index 5809126..64e8167 100644 --- a/src/rfc2131.c +++ b/src/rfc2131.c @@ -949,7 +949,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, mess->siaddr = a_record_from_hosts(boot->tftp_sname, now); if (boot->file) - strncpy((char *)mess->file, boot->file, sizeof(mess->file)-1); + safe_strncpy((char *)mess->file, boot->file, sizeof(mess->file)); } option_put(mess, end, OPTION_MESSAGE_TYPE, 1, @@ -2360,7 +2360,7 @@ static void do_options(struct dhcp_context *context, in_list(req_options, OPTION_SNAME)) option_put_string(mess, end, OPTION_SNAME, boot->sname, 1); else - strncpy((char *)mess->sname, boot->sname, sizeof(mess->sname)-1); + safe_strncpy((char *)mess->sname, boot->sname, sizeof(mess->sname)); } if (boot->file) @@ -2370,7 +2370,7 @@ static void do_options(struct dhcp_context *context, in_list(req_options, OPTION_FILENAME)) option_put_string(mess, end, OPTION_FILENAME, boot->file, 1); else - strncpy((char *)mess->file, boot->file, sizeof(mess->file)-1); + safe_strncpy((char *)mess->file, boot->file, sizeof(mess->file)); } if (boot->next_server.s_addr) @@ -2387,14 +2387,14 @@ static void do_options(struct dhcp_context *context, if ((!req_options || !in_list(req_options, OPTION_FILENAME)) && (opt = option_find2(OPTION_FILENAME)) && !(opt->flags & DHOPT_FORCE)) { - strncpy((char *)mess->file, (char *)opt->val, sizeof(mess->file)-1); + safe_strncpy((char *)mess->file, (char *)opt->val, sizeof(mess->file)); done_file = 1; } if ((!req_options || !in_list(req_options, OPTION_SNAME)) && (opt = option_find2(OPTION_SNAME)) && !(opt->flags & DHOPT_FORCE)) { - strncpy((char *)mess->sname, (char *)opt->val, sizeof(mess->sname)-1); + safe_strncpy((char *)mess->sname, (char *)opt->val, sizeof(mess->sname)); done_server = 1; } diff --git a/src/tftp.c b/src/tftp.c index bccca69..f2eccbc 100644 --- a/src/tftp.c +++ b/src/tftp.c @@ -234,7 +234,7 @@ void tftp_request(struct listener *listen, time_t now) #endif } - strncpy(ifr.ifr_name, name, IF_NAMESIZE); + safe_strncpy(ifr.ifr_name, name, IF_NAMESIZE); if (ioctl(listen->tftpfd, SIOCGIFMTU, &ifr) != -1) { mtu = ifr.ifr_mtu; diff --git a/src/util.c b/src/util.c index b27d50e..2b5c4fd 100644 --- a/src/util.c +++ b/src/util.c @@ -281,7 +281,18 @@ void *safe_malloc(size_t size) die(_("could not get memory"), NULL, EC_NOMEM); return ret; -} +} + +/* Ensure limited size string is always terminated. + * Can be replaced by (void)strlcpy() on some platforms */ +void safe_strncpy(char *dest, const char *src, size_t size) +{ + if (size) + { + dest[size-1] = '\0'; + strncpy(dest, src, size-1); + } +} void safe_pipe(int *fd, int read_noblock) {