From 3042a287bcd06f4c0a53c3bb7be0814581ed131d Mon Sep 17 00:00:00 2001 From: jcmichelou Date: Sat, 6 May 2017 16:56:32 +0200 Subject: [PATCH] Fix a problem with the use of control structures in sendmsg / recvmsg. These are not struct sockaddr, they are ip_pktinfo or ipv6_pktinfo, so local_interface->addr.size is irrelevant, even causing an overflow with writing zero bytes outside of the control structure on the stack, even though it happens to work because the buffer is large enough. Prevent invalid addresses from being used as source address with sendmsg. On linux, they are just ignored, so it is the same as not specifying one, but on windows they cause an error. In both case it is simpler not to include any control info if the intent is to let the interface be chosen by the network stack. --- src/coap_io.c | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/src/coap_io.c b/src/coap_io.c index 08d55b9e..bd16bc22 100644 --- a/src/coap_io.c +++ b/src/coap_io.c @@ -284,8 +284,8 @@ coap_network_send(struct coap_context_t *context UNUSED_PARAM, (struct coap_endpoint_t *)local_interface; #ifndef WITH_CONTIKI - /* a buffer large enough to hold all protocol address types */ - char buf[CMSG_LEN(sizeof(struct sockaddr_storage))]; + /* a buffer large enough to hold all packet info types, ipv6 is the largest */ + char buf[CMSG_SPACE(sizeof(struct in6_pktinfo))]; (void)context; #ifdef _WIN32 DWORD dwNumberOfBytesSent = 0; @@ -307,7 +307,7 @@ coap_network_send(struct coap_context_t *context UNUSED_PARAM, mhdr.msg_iov = iov; mhdr.msg_iovlen = 1; - switch (dst->addr.sa.sa_family) { + if ( !coap_address_isany(&local_interface->addr) && !coap_is_mcast(&local_interface->addr) ) switch (dst->addr.sa.sa_family) { case AF_INET6: { struct cmsghdr *cmsg; struct in6_pktinfo *pktinfo; @@ -324,17 +324,7 @@ coap_network_send(struct coap_context_t *context UNUSED_PARAM, memset(pktinfo, 0, sizeof(struct in6_pktinfo)); pktinfo->ipi6_ifindex = ep->ifindex; - if (coap_is_mcast(&local_interface->addr)) { - /* We cannot send with multicast address as source address - * and hence let the kernel pick the outgoing interface. */ - pktinfo->ipi6_ifindex = 0; - memset(&pktinfo->ipi6_addr, 0, sizeof(pktinfo->ipi6_addr)); - } else { - pktinfo->ipi6_ifindex = ep->ifindex; - memcpy(&pktinfo->ipi6_addr, - &local_interface->addr.addr.sin6.sin6_addr, - local_interface->addr.size); - } + memcpy( &pktinfo->ipi6_addr, &local_interface->addr.addr.sin6.sin6_addr, sizeof(pktinfo->ipi6_addr) ); break; } case AF_INET: { @@ -353,17 +343,8 @@ coap_network_send(struct coap_context_t *context UNUSED_PARAM, pktinfo = (struct in_pktinfo *)CMSG_DATA(cmsg); memset(pktinfo, 0, sizeof(struct in_pktinfo)); - if (coap_is_mcast(&local_interface->addr)) { - /* We cannot send with multicast address as source address - * and hence let the kernel pick the outgoing interface. */ - pktinfo->ipi_ifindex = 0; - memset(&pktinfo->ipi_spec_dst, 0, sizeof(pktinfo->ipi_spec_dst)); - } else { - pktinfo->ipi_ifindex = ep->ifindex; - memcpy(&pktinfo->ipi_spec_dst, - &local_interface->addr.addr.sin.sin_addr, - local_interface->addr.size); - } + pktinfo->ipi_ifindex = ep->ifindex; + memcpy( &pktinfo->ipi_spec_dst, &local_interface->addr.addr.sin.sin_addr, sizeof(pktinfo->ipi_spec_dst) ); #endif /* IP_PKTINFO */ break; } @@ -475,7 +456,8 @@ coap_network_read(coap_endpoint_t *ep, coap_packet_t **packet) { #endif #if !defined(WITH_CONTIKI) && !defined(WITH_LWIP) - char buf[CMSG_LEN(sizeof(struct sockaddr_storage))]; + /* a buffer large enough to hold all packet info types, ipv6 is the largest */ + char buf[CMSG_SPACE(sizeof(struct in6_pktinfo))]; struct msghdr mhdr; struct iovec iov[1]; #endif @@ -507,7 +489,6 @@ coap_network_read(coap_endpoint_t *ep, coap_packet_t **packet) { mhdr.msg_control = buf; mhdr.msg_controllen = sizeof(buf); - assert(sizeof(buf) == CMSG_LEN(sizeof(struct sockaddr_storage))); #if defined(_WIN32) if ( !lpWSARecvMsg ) {