From f7a6418d47d141f4543aa01253f64f60ffdd2e17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 14 May 2024 18:04:31 +0200 Subject: [PATCH 1/4] libsystemd-network: remove double initialization --- src/libsystemd-network/icmp6-util.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libsystemd-network/icmp6-util.c b/src/libsystemd-network/icmp6-util.c index a28f175b5e..75a6489967 100644 --- a/src/libsystemd-network/icmp6-util.c +++ b/src/libsystemd-network/icmp6-util.c @@ -121,7 +121,7 @@ int icmp6_receive( /* This needs to be initialized with zero. See #20741. */ CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int)) + /* ttl */ CMSG_SPACE_TIMEVAL) control = {}; - struct iovec iov = {}; + struct iovec iov = { buffer, size }; union sockaddr_union sa = {}; struct msghdr msg = { .msg_name = &sa.sa, @@ -133,8 +133,6 @@ int icmp6_receive( }; ssize_t len; - iov = IOVEC_MAKE(buffer, size); - len = recvmsg_safe(fd, &msg, MSG_DONTWAIT); if (len < 0) return (int) len; From caa4bfd4688f0997d724acd8be759f17b7ae3291 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 14 May 2024 18:43:29 +0200 Subject: [PATCH 2/4] libsystemd-network: check size of icmpv6 packets Coverity was complaining that we use the received packet size as a loop bound without checking. This is indeed a bit iffy, because depending on how the host is configured, the packet could be rather large. Let's refuse anything more than the standard size early to prevent suspicious activity. Resolves coverity CID#1534892, CID#1543949. --- src/libsystemd-network/icmp6-packet.c | 5 +++++ src/libsystemd-network/icmp6-packet.h | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/libsystemd-network/icmp6-packet.c b/src/libsystemd-network/icmp6-packet.c index 21d744beaa..f447549dc8 100644 --- a/src/libsystemd-network/icmp6-packet.c +++ b/src/libsystemd-network/icmp6-packet.c @@ -88,6 +88,11 @@ static int icmp6_packet_verify(ICMP6Packet *p) { if (hdr->icmp6_code != 0) return -EBADMSG; + /* Drop any overly large packets early. We are not interested in jumbograms, + * which could cause excessive processing. */ + if (p->raw_size > ICMP6_MAX_NORMAL_PAYLOAD_SIZE) + return -EMSGSIZE; + return 0; } diff --git a/src/libsystemd-network/icmp6-packet.h b/src/libsystemd-network/icmp6-packet.h index 16f354fc95..b402255806 100644 --- a/src/libsystemd-network/icmp6-packet.h +++ b/src/libsystemd-network/icmp6-packet.h @@ -21,6 +21,10 @@ ICMP6Packet* icmp6_packet_ref(ICMP6Packet *p); ICMP6Packet* icmp6_packet_unref(ICMP6Packet *p); DEFINE_TRIVIAL_CLEANUP_FUNC(ICMP6Packet*, icmp6_packet_unref); +/* IPv6 Header is 40 bytes and reserves 2 bytes to represent the Payload Length. Thus, the max payload size, + * including extension headers, is 65535 bytes (2^16 - 1). Jumbograms can be larger (2^32 - 1). */ +#define ICMP6_MAX_NORMAL_PAYLOAD_SIZE 65535 + int icmp6_packet_set_sender_address(ICMP6Packet *p, const struct in6_addr *addr); int icmp6_packet_get_sender_address(ICMP6Packet *p, struct in6_addr *ret); int icmp6_packet_get_timestamp(ICMP6Packet *p, clockid_t clock, usec_t *ret); From b17b91db8060b7ac65e20834d3b4c40d4985a9ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 14 May 2024 18:51:57 +0200 Subject: [PATCH 3/4] libsystemd-network: use normal style for error checking We generally use a flat list. The switch stmt was generating excessive indentation. --- src/libsystemd-network/icmp6-packet.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libsystemd-network/icmp6-packet.c b/src/libsystemd-network/icmp6-packet.c index f447549dc8..02865a40c2 100644 --- a/src/libsystemd-network/icmp6-packet.c +++ b/src/libsystemd-network/icmp6-packet.c @@ -113,20 +113,14 @@ int icmp6_packet_receive(int fd, ICMP6Packet **ret) { return -ENOMEM; r = icmp6_receive(fd, p->raw_packet, p->raw_size, &p->sender_address, &p->timestamp); + if (r == -EADDRNOTAVAIL) + return log_debug_errno(r, "ICMPv6: Received a packet from neither link-local nor null address."); + if (r == -EMULTIHOP) + return log_debug_errno(r, "ICMPv6: Received a packet with an invalid hop limit."); + if (r == -EPFNOSUPPORT) + return log_debug_errno(r, "ICMPv6: Received a packet with an invalid source address."); if (r < 0) - switch (r) { - case -EADDRNOTAVAIL: - return log_debug_errno(r, "ICMPv6: Received a packet from neither link-local nor null address."); - - case -EMULTIHOP: - return log_debug_errno(r, "ICMPv6: Received a packet with an invalid hop limit."); - - case -EPFNOSUPPORT: - return log_debug_errno(r, "ICMPv6: Received a packet with an invalid source address."); - - default: - return log_debug_errno(r, "ICMPv6: Unexpected error while receiving a packet: %m"); - } + return log_debug_errno(r, "ICMPv6: Unexpected error while receiving a packet: %m"); r = icmp6_packet_verify(p); if (r < 0) From 8e41e460e07732fbc584ab6b3572e403a6a76d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 14 May 2024 17:59:09 +0200 Subject: [PATCH 4/4] test: rename file with stub functions icmp6-util-linux.c sounds like a specialized implementation of the functions in icmp6-util.c. But it's just a set of stub versions used in tests. Rename the file to make this more obvious. --- src/libsystemd-network/fuzz-ndisc-rs.c | 2 +- .../{icmp6-util-unix.c => icmp6-test-util.c} | 2 +- .../{icmp6-util-unix.h => icmp6-test-util.h} | 0 src/libsystemd-network/meson.build | 6 +++--- src/libsystemd-network/test-ndisc-ra.c | 2 +- src/libsystemd-network/test-ndisc-rs.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) rename src/libsystemd-network/{icmp6-util-unix.c => icmp6-test-util.c} (97%) rename src/libsystemd-network/{icmp6-util-unix.h => icmp6-test-util.h} (100%) diff --git a/src/libsystemd-network/fuzz-ndisc-rs.c b/src/libsystemd-network/fuzz-ndisc-rs.c index 780a5d7f5a..e6ee76895f 100644 --- a/src/libsystemd-network/fuzz-ndisc-rs.c +++ b/src/libsystemd-network/fuzz-ndisc-rs.c @@ -11,7 +11,7 @@ #include "fd-util.h" #include "fuzz.h" #include "icmp6-packet.h" -#include "icmp6-util-unix.h" +#include "icmp6-test-util.h" #include "ndisc-internal.h" #include "ndisc-option.h" #include "socket-util.h" diff --git a/src/libsystemd-network/icmp6-util-unix.c b/src/libsystemd-network/icmp6-test-util.c similarity index 97% rename from src/libsystemd-network/icmp6-util-unix.c rename to src/libsystemd-network/icmp6-test-util.c index d6d505717d..3c781095bc 100644 --- a/src/libsystemd-network/icmp6-util-unix.c +++ b/src/libsystemd-network/icmp6-test-util.c @@ -5,7 +5,7 @@ #include #include "fd-util.h" -#include "icmp6-util-unix.h" +#include "icmp6-test-util.h" int test_fd[2] = EBADF_PAIR; diff --git a/src/libsystemd-network/icmp6-util-unix.h b/src/libsystemd-network/icmp6-test-util.h similarity index 100% rename from src/libsystemd-network/icmp6-util-unix.h rename to src/libsystemd-network/icmp6-test-util.h diff --git a/src/libsystemd-network/meson.build b/src/libsystemd-network/meson.build index fdd8806a23..718495cd8e 100644 --- a/src/libsystemd-network/meson.build +++ b/src/libsystemd-network/meson.build @@ -91,13 +91,13 @@ executables += [ network_test_template + { 'sources' : files( 'test-ndisc-ra.c', - 'icmp6-util-unix.c', + 'icmp6-test-util.c', ), }, network_test_template + { 'sources' : files( 'test-ndisc-rs.c', - 'icmp6-util-unix.c', + 'icmp6-test-util.c', ), }, network_test_template + { @@ -125,7 +125,7 @@ executables += [ network_fuzz_template + { 'sources' : files( 'fuzz-ndisc-rs.c', - 'icmp6-util-unix.c', + 'icmp6-test-util.c', ), }, ] diff --git a/src/libsystemd-network/test-ndisc-ra.c b/src/libsystemd-network/test-ndisc-ra.c index 8be7351a09..14c9164585 100644 --- a/src/libsystemd-network/test-ndisc-ra.c +++ b/src/libsystemd-network/test-ndisc-ra.c @@ -11,7 +11,7 @@ #include "alloc-util.h" #include "hexdecoct.h" -#include "icmp6-util-unix.h" +#include "icmp6-test-util.h" #include "socket-util.h" #include "strv.h" #include "tests.h" diff --git a/src/libsystemd-network/test-ndisc-rs.c b/src/libsystemd-network/test-ndisc-rs.c index 5ad2c92b49..66aad2600a 100644 --- a/src/libsystemd-network/test-ndisc-rs.c +++ b/src/libsystemd-network/test-ndisc-rs.c @@ -13,7 +13,7 @@ #include "fd-util.h" #include "hexdecoct.h" #include "icmp6-packet.h" -#include "icmp6-util-unix.h" +#include "icmp6-test-util.h" #include "socket-util.h" #include "strv.h" #include "ndisc-internal.h"