ndisc: reject non-zero ICMPv6 codes in parsers

NDisc packets received through the socket path are filtered before
parser dispatch, but parser entry points should still reject malformed
packet bytes instead of asserting on them. Return EBADMSG for non-zero
ICMPv6 code values in RA, NA, and Redirect messages.

Follow-up for c34cb1d6451dd9fcd36e1c08c553ca7f25e9d83b
Follow-up for 696eb2b8de980a2b79c1de7fbf12195936b00441
Follow-up for 44e8cf303b1e54752637725d55d01811e05ed482

diff --git a/src/libsystemd-network/sd-ndisc-neighbor.c b/src/libsystemd-network/sd-ndisc-neighbor.c
index 5ce1da06da3651d4a761643968e3ffc38d8a3a5b..a5de33db8997e9171560b7dce7f7de6310b1e32a 100644 (file)
--- a/src/libsystemd-network/sd-ndisc-neighbor.c
+++ b/src/libsystemd-network/sd-ndisc-neighbor.c
@@ -52,7 +52,9 @@ int ndisc_neighbor_parse(sd_ndisc *nd, sd_ndisc_neighbor *na) {
         /* Neighbor advertisement packets are neatly aligned to 64-bit boundaries, hence we can access them directly */
         const struct nd_neighbor_advert *a = (const struct nd_neighbor_advert*) na->packet->raw_packet;
         assert(a->nd_na_type == ND_NEIGHBOR_ADVERT);
-        assert(a->nd_na_code == 0);
+        if (a->nd_na_code != 0)
+                return log_ndisc_errno(nd, SYNTHETIC_ERRNO(EBADMSG),
+                                       "Received Neighbor Advertisement with non-zero code, ignoring datagram.");
 
         na->flags = a->nd_na_flags_reserved; /* the first 3 bits */
         na->target_address = a->nd_na_target;

diff --git a/src/libsystemd-network/sd-ndisc-redirect.c b/src/libsystemd-network/sd-ndisc-redirect.c
index 4845adbd7bbeda731c20c7a67597276b121eee53..fcfefc5ca5b129035a988c7947f0053f3d7fd656 100644 (file)
--- a/src/libsystemd-network/sd-ndisc-redirect.c
+++ b/src/libsystemd-network/sd-ndisc-redirect.c
@@ -52,7 +52,9 @@ int ndisc_redirect_parse(sd_ndisc *nd, sd_ndisc_redirect *rd) {
 
         const struct nd_redirect *a = (const struct nd_redirect*) rd->packet->raw_packet;
         assert(a->nd_rd_type == ND_REDIRECT);
-        assert(a->nd_rd_code == 0);
+        if (a->nd_rd_code != 0)
+                return log_ndisc_errno(nd, SYNTHETIC_ERRNO(EBADMSG),
+                                       "Received Redirect message with non-zero code, ignoring datagram.");
 
         rd->target_address = a->nd_rd_target;
         rd->destination_address = a->nd_rd_dst;

diff --git a/src/libsystemd-network/sd-ndisc-router.c b/src/libsystemd-network/sd-ndisc-router.c
index 9d6181b3554d52d1db4887f15d3c84207d409191..fcce89cc9b7d95f0549c99f62f914fc9922d9eac 100644 (file)
--- a/src/libsystemd-network/sd-ndisc-router.c
+++ b/src/libsystemd-network/sd-ndisc-router.c
@@ -108,7 +108,9 @@ int ndisc_router_parse(sd_ndisc *nd, sd_ndisc_router *rt) {
 
         a = (const struct nd_router_advert*) rt->packet->raw_packet;
         assert(a->nd_ra_type == ND_ROUTER_ADVERT);
-        assert(a->nd_ra_code == 0);
+        if (a->nd_ra_code != 0)
+                return log_ndisc_errno(nd, SYNTHETIC_ERRNO(EBADMSG),
+                                       "Received Router Advertisement with non-zero code, ignoring.");
 
         rt->hop_limit = a->nd_ra_curhoplimit;
         rt->flags = a->nd_ra_flags_reserved; /* the first 8 bits */

diff --git a/src/libsystemd-network/test-ndisc-rs.c b/src/libsystemd-network/test-ndisc-rs.c
index 9c1e6f03a4c7f87ab218708822363cc7787b9616..86b7c07bdbf6a6d5b6a2a4fe823a5b2d23ac9887 100644 (file)
--- a/src/libsystemd-network/test-ndisc-rs.c
+++ b/src/libsystemd-network/test-ndisc-rs.c
@@ -15,6 +15,9 @@
 #include "icmp6-test-util.h"
 #include "in-addr-util.h"
 #include "ndisc-internal.h"
+#include "ndisc-neighbor-internal.h"
+#include "ndisc-redirect-internal.h"
+#include "ndisc-router-internal.h"
 #include "strv.h"
 #include "tests.h"
 
@@ -22,6 +25,59 @@ static struct ether_addr mac_addr = {
         .ether_addr_octet = {'A', 'B', 'C', '1', '2', '3'}
 };
 
+static ICMP6Packet* make_packet(const void *raw, size_t size) {
+        ICMP6Packet *packet;
+
+        assert(raw);
+
+        packet = malloc0(offsetof(ICMP6Packet, raw_packet) + size);
+        ASSERT_NOT_NULL(packet);
+
+        packet->n_ref = 1;
+        packet->raw_size = size;
+        memcpy(packet->raw_packet, raw, size);
+
+        return packet;
+}
+
+TEST(parse_rejects_nonzero_code) {
+        static const struct nd_router_advert router_advertisement = {
+                .nd_ra_type = ND_ROUTER_ADVERT,
+                .nd_ra_code = 1,
+        };
+        static const struct nd_neighbor_advert neighbor_advertisement = {
+                .nd_na_type = ND_NEIGHBOR_ADVERT,
+                .nd_na_code = 1,
+        };
+        static const struct nd_redirect redirect = {
+                .nd_rd_type = ND_REDIRECT,
+                .nd_rd_code = 1,
+        };
+
+        _cleanup_(icmp6_packet_unrefp) ICMP6Packet *packet =
+                make_packet(&router_advertisement, sizeof(router_advertisement));
+        _cleanup_(sd_ndisc_router_unrefp) sd_ndisc_router *router =
+                ASSERT_PTR(ndisc_router_new(packet));
+
+        ASSERT_ERROR(ndisc_router_parse(/* nd= */ NULL, router), EBADMSG);
+        router = sd_ndisc_router_unref(router);
+        packet = icmp6_packet_unref(packet);
+
+        packet = make_packet(&neighbor_advertisement, sizeof(neighbor_advertisement));
+        _cleanup_(sd_ndisc_neighbor_unrefp) sd_ndisc_neighbor *neighbor =
+                ASSERT_PTR(ndisc_neighbor_new(packet));
+
+        ASSERT_ERROR(ndisc_neighbor_parse(/* nd= */ NULL, neighbor), EBADMSG);
+        neighbor = sd_ndisc_neighbor_unref(neighbor);
+        packet = icmp6_packet_unref(packet);
+
+        packet = make_packet(&redirect, sizeof(redirect));
+        _cleanup_(sd_ndisc_redirect_unrefp) sd_ndisc_redirect *rd =
+                ASSERT_PTR(ndisc_redirect_new(packet));
+
+        ASSERT_ERROR(ndisc_redirect_parse(/* nd= */ NULL, rd), EBADMSG);
+}
+
 static void router_dump(sd_ndisc_router *rt) {
         struct in6_addr addr;
         uint8_t hop_limit;