[Backport v4.1-branch] net: icmp: Verify the address family before calling the callback

include/zephyr/net/icmp.h

@@ -93,6 +93,9 @@ struct net_icmp_ctx {
 	/** Opaque user supplied data */
 	void *user_data;
 
+	/** Address family the handler is registered for */
+	uint8_t family;
+
 	/** ICMP type of the response we are waiting */
 	uint8_t type;
 
@@ -157,12 +160,13 @@ struct net_icmp_ping_params {
  *        system.
  *
  * @param ctx ICMP context used in this request.
+ * @param family Address family the context is using.
  * @param type Type of ICMP message we are handling.
  * @param code Code of ICMP message we are handling.
  * @param handler Callback function that is called when a response is received.
  */
-int net_icmp_init_ctx(struct net_icmp_ctx *ctx, uint8_t type, uint8_t code,
-		      net_icmp_handler_t handler);
+int net_icmp_init_ctx(struct net_icmp_ctx *ctx, uint8_t family, uint8_t type,
+		      uint8_t code, net_icmp_handler_t handler);
 
 /**
  * @brief Cleanup the ICMP context structure. This will unregister the ICMP handler

subsys/net/ip/icmp.c

@@ -50,16 +50,22 @@ static sys_slist_t offload_handlers = SYS_SLIST_STATIC_INIT(&offload_handlers);
 
 #define PKT_WAIT_TIME K_SECONDS(1)
 
-int net_icmp_init_ctx(struct net_icmp_ctx *ctx, uint8_t type, uint8_t code,
-		      net_icmp_handler_t handler)
+int net_icmp_init_ctx(struct net_icmp_ctx *ctx, uint8_t family, uint8_t type,
+		      uint8_t code, net_icmp_handler_t handler)
 {
 	if (ctx == NULL || handler == NULL) {
 		return -EINVAL;
 	}
 
+	if (family != AF_INET && family != AF_INET6) {
+		NET_ERR("Wrong address family");
+		return -EINVAL;
+	}
+
 	memset(ctx, 0, sizeof(struct net_icmp_ctx));
 
 	ctx->handler = handler;
+	ctx->family = family;
 	ctx->type = type;
 	ctx->code = code;
 
@@ -511,6 +517,10 @@ static int icmp_call_handlers(struct net_pkt *pkt,
 	k_mutex_lock(&lock, K_FOREVER);
 
 	SYS_SLIST_FOR_EACH_CONTAINER(&handlers, ctx, node) {
+		if (ip_hdr->family != ctx->family) {
+			continue;
+		}
+
 		if (ctx->type == icmp_hdr->type &&
 		    (ctx->code == icmp_hdr->code || ctx->code == 0U)) {
 			/* Do not use a handler that is expecting data from different

subsys/net/ip/icmpv4.c

@@ -762,14 +762,15 @@ void net_icmpv4_init(void)
 	static struct net_icmp_ctx ctx;
 	int ret;
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV4_ECHO_REQUEST, 0, icmpv4_handle_echo_request);
+	ret = net_icmp_init_ctx(&ctx, AF_INET, NET_ICMPV4_ECHO_REQUEST, 0,
+				icmpv4_handle_echo_request);
 	if (ret < 0) {
 		NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV4_ECHO_REQUEST),
 			ret);
 	}
 
 #if defined(CONFIG_NET_IPV4_PMTU)
-	ret = net_icmp_init_ctx(&dst_unreach_ctx, NET_ICMPV4_DST_UNREACH, 0,
+	ret = net_icmp_init_ctx(&dst_unreach_ctx, AF_INET, NET_ICMPV4_DST_UNREACH, 0,
 				icmpv4_handle_dst_unreach);
 	if (ret < 0) {
 		NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV4_DST_UNREACH),

subsys/net/ip/icmpv6.c

@@ -384,7 +384,8 @@ void net_icmpv6_init(void)
 	static struct net_icmp_ctx ctx;
 	int ret;
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV6_ECHO_REQUEST, 0, icmpv6_handle_echo_request);
+	ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_ECHO_REQUEST, 0,
+				icmpv6_handle_echo_request);
 	if (ret < 0) {
 		NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_ECHO_REQUEST),
 			ret);

subsys/net/ip/ipv6_mld.c

@@ -461,7 +461,7 @@ void net_ipv6_mld_init(void)
 	static struct net_icmp_ctx ctx;
 	int ret;
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV6_MLD_QUERY, 0, handle_mld_query);
+	ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_MLD_QUERY, 0, handle_mld_query);
 	if (ret < 0) {
 		NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_MLD_QUERY),
 			ret);

subsys/net/ip/ipv6_nbr.c

@@ -2865,13 +2865,13 @@ void net_ipv6_nbr_init(void)
 	int ret;
 
 #if defined(CONFIG_NET_IPV6_NBR_CACHE)
-	ret = net_icmp_init_ctx(&ns_ctx, NET_ICMPV6_NS, 0, handle_ns_input);
+	ret = net_icmp_init_ctx(&ns_ctx, AF_INET6, NET_ICMPV6_NS, 0, handle_ns_input);
 	if (ret < 0) {
 		NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_NS),
 			ret);
 	}
 
-	ret = net_icmp_init_ctx(&na_ctx, NET_ICMPV6_NA, 0, handle_na_input);
+	ret = net_icmp_init_ctx(&na_ctx, AF_INET6, NET_ICMPV6_NA, 0, handle_na_input);
 	if (ret < 0) {
 		NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_NA),
 			ret);
@@ -2880,7 +2880,7 @@ void net_ipv6_nbr_init(void)
 	k_work_init_delayable(&ipv6_ns_reply_timer, ipv6_ns_reply_timeout);
 #endif
 #if defined(CONFIG_NET_IPV6_ND)
-	ret = net_icmp_init_ctx(&ra_ctx, NET_ICMPV6_RA, 0, handle_ra_input);
+	ret = net_icmp_init_ctx(&ra_ctx, AF_INET6, NET_ICMPV6_RA, 0, handle_ra_input);
 	if (ret < 0) {
 		NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_RA),
 			ret);
@@ -2891,7 +2891,7 @@ void net_ipv6_nbr_init(void)
 #endif
 
 #if defined(CONFIG_NET_IPV6_PMTU)
-	ret = net_icmp_init_ctx(&ptb_ctx, NET_ICMPV6_PACKET_TOO_BIG, 0, handle_ptb_input);
+	ret = net_icmp_init_ctx(&ptb_ctx, AF_INET6, NET_ICMPV6_PACKET_TOO_BIG, 0, handle_ptb_input);
 	if (ret < 0) {
 		NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_PACKET_TOO_BIG),
 			ret);

subsys/net/lib/dhcpv4/dhcpv4_server.c

@@ -879,7 +879,7 @@ static int echo_reply_handler(struct net_icmp_ctx *icmp_ctx,
 
 static int dhcpv4_server_probing_init(struct dhcpv4_server_ctx *ctx)
 {
-	return net_icmp_init_ctx(&ctx->probe_ctx.icmp_ctx,
+	return net_icmp_init_ctx(&ctx->probe_ctx.icmp_ctx, AF_INET,
 				 NET_ICMPV4_ECHO_REPLY, 0,
 				 echo_reply_handler);
 }

subsys/net/lib/shell/ping.c

@@ -456,7 +456,7 @@ static int cmd_net_ping(const struct shell *sh, size_t argc, char *argv[])
 	    net_addr_pton(AF_INET6, host, &ping_ctx.addr6.sin6_addr) == 0) {
 		ping_ctx.addr6.sin6_family = AF_INET6;
 
-		ret = net_icmp_init_ctx(&ping_ctx.icmp, NET_ICMPV6_ECHO_REPLY, 0,
+		ret = net_icmp_init_ctx(&ping_ctx.icmp, AF_INET6, NET_ICMPV6_ECHO_REPLY, 0,
 					handle_ipv6_echo_reply);
 		if (ret < 0) {
 			PR_WARNING("Cannot initialize ICMP context for %s\n", "IPv6");
@@ -466,7 +466,7 @@ static int cmd_net_ping(const struct shell *sh, size_t argc, char *argv[])
 		   net_addr_pton(AF_INET, host, &ping_ctx.addr4.sin_addr) == 0) {
 		ping_ctx.addr4.sin_family = AF_INET;
 
-		ret = net_icmp_init_ctx(&ping_ctx.icmp, NET_ICMPV4_ECHO_REPLY, 0,
+		ret = net_icmp_init_ctx(&ping_ctx.icmp, AF_INET, NET_ICMPV4_ECHO_REPLY, 0,
 					handle_ipv4_echo_reply);
 		if (ret < 0) {
 			PR_WARNING("Cannot initialize ICMP context for %s\n", "IPv4");

subsys/net/lib/zperf/zperf_shell.c

@@ -679,7 +679,7 @@ static void send_ping(const struct shell *sh,
 	struct net_icmp_ctx ctx;
 	int ret;
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV6_ECHO_REPLY, 0, ping_handler);
+	ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_ECHO_REPLY, 0, ping_handler);
 	if (ret < 0) {
 		shell_fprintf(sh, SHELL_WARNING, "Cannot send ping (%d)\n", ret);
 		return;

tests/boards/espressif/ethernet/src/main.c

@@ -97,7 +97,7 @@ ZTEST(ethernet, test_icmp_check)
 	gw_addr_4 = net_if_ipv4_get_gw(iface);
 	zassert_not_equal(gw_addr_4.s_addr, 0, "Gateway address is not set");
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV4_ECHO_REPLY, 0, icmp_event);
+	ret = net_icmp_init_ctx(&ctx, AF_INET, NET_ICMPV4_ECHO_REPLY, 0, icmp_event);
 	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
 
 	dst4.sin_family = AF_INET;

tests/boards/espressif/wifi/src/main.c

@@ -282,7 +282,7 @@ ZTEST(wifi, test_2_icmp)
 	gw_addr_4 = net_if_ipv4_get_gw(wifi_ctx.iface);
 	zassert_not_equal(gw_addr_4.s_addr, 0, "Gateway address is not set");
 
-	ret = net_icmp_init_ctx(&icmp_ctx, NET_ICMPV4_ECHO_REPLY, 0, icmp_event);
+	ret = net_icmp_init_ctx(&icmp_ctx, AF_INET, NET_ICMPV4_ECHO_REPLY, 0, icmp_event);
 	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
 
 	dst4.sin_family = AF_INET;

tests/net/checksum_offload/src/main.c

@@ -876,7 +876,7 @@ static void test_tx_chksum_icmp_frag(sa_family_t family, bool offloaded)
 
 	test_icmp_init(family, offloaded, &dst_addr, &iface);
 
-	ret = net_icmp_init_ctx(&ctx, 0, 0, dummy_icmp_handler);
+	ret = net_icmp_init_ctx(&ctx, family, 0, 0, dummy_icmp_handler);
 	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
 
 	test_started = true;
@@ -1212,7 +1212,7 @@ static void test_rx_chksum_icmp_frag(sa_family_t family, bool offloaded)
 
 	test_icmp_init(family, offloaded, &dst_addr, &iface);
 
-	ret = net_icmp_init_ctx(&ctx,
+	ret = net_icmp_init_ctx(&ctx, family,
 				family == AF_INET6 ? NET_ICMPV6_ECHO_REPLY :
 						     NET_ICMPV4_ECHO_REPLY,
 				0, icmp_handler);
@@ -1269,7 +1269,7 @@ static void test_rx_chksum_icmp_frag_bad(sa_family_t family, bool offloaded)
 
 	test_icmp_init(family, offloaded, &dst_addr, &iface);
 
-	ret = net_icmp_init_ctx(&ctx,
+	ret = net_icmp_init_ctx(&ctx, family,
 				family == AF_INET6 ? NET_ICMPV6_ECHO_REPLY :
 						     NET_ICMPV4_ECHO_REPLY,
 				0, icmp_handler);

tests/net/icmp/src/main.c

@@ -465,7 +465,7 @@ ZTEST(icmp_tests, test_icmpv6_echo_request)
 		return;
 	}
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV6_ECHO_REPLY, 0, icmp_handler);
+	ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_ECHO_REPLY, 0, icmp_handler);
 	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
 
 	dst6.sin6_family = AF_INET6;
@@ -508,7 +508,7 @@ ZTEST(icmp_tests, test_icmpv4_echo_request)
 		return;
 	}
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV4_ECHO_REPLY, 0, icmp_handler);
+	ret = net_icmp_init_ctx(&ctx, AF_INET, NET_ICMPV4_ECHO_REPLY, 0, icmp_handler);
 	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
 
 	dst4.sin_family = AF_INET;
@@ -549,7 +549,7 @@ ZTEST(icmp_tests, test_offload_icmpv4_echo_request)
 	struct net_icmp_ctx ctx;
 	int ret;
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV4_ECHO_REPLY, 0, icmp_handler);
+	ret = net_icmp_init_ctx(&ctx, AF_INET, NET_ICMPV4_ECHO_REPLY, 0, icmp_handler);
 	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
 
 	dst4.sin_family = AF_INET;
@@ -588,7 +588,7 @@ ZTEST(icmp_tests, test_offload_icmpv6_echo_request)
 	struct net_icmp_ctx ctx;
 	int ret;
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV6_ECHO_REPLY, 0, icmp_handler);
+	ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_ECHO_REPLY, 0, icmp_handler);
 	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
 
 	dst6.sin6_family = AF_INET6;
@@ -620,6 +620,126 @@ ZTEST(icmp_tests, test_offload_icmpv6_echo_request)
 #endif
 #endif /* CONFIG_NET_OFFLOADING_SUPPORT */
 
+/* Need to have both IPv4/IPv6 for those */
+#if defined(CONFIG_NET_IPV4) && defined(CONFIG_NET_IPV6)
+static K_SEM_DEFINE(test_req_sem, 0, 1);
+
+static int icmp_request_handler(struct net_icmp_ctx *ctx,
+				struct net_pkt *pkt,
+				struct net_icmp_ip_hdr *hdr,
+				struct net_icmp_hdr *icmp_hdr,
+				void *user_data)
+{
+	k_sem_give(&test_req_sem);
+
+	return 0;
+}
+
+ZTEST(icmp_tests, test_malformed_icmpv6_echo_request_on_ipv4)
+{
+	struct in_addr dst4 = { 0 };
+	const struct in_addr *src4;
+	struct net_icmp_ctx ctx;
+	struct net_if *iface;
+	struct net_pkt *pkt;
+	int ret;
+
+	k_sem_reset(&test_req_sem);
+
+	ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_ECHO_REQUEST, 0,
+				icmp_request_handler);
+	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
+
+	memcpy(&dst4, &recv_addr_4, sizeof(recv_addr_4));
+
+	/* Prepare malformed NET_ICMPV6_ECHO_REQUEST on IPv4 packet */
+	iface = net_if_ipv4_select_src_iface(&dst4);
+	zassert_not_null(iface, "NULL iface");
+
+	src4 = net_if_ipv4_select_src_addr(iface, &dst4);
+	zassert_not_null(src4, "NULL addr");
+
+	pkt = net_pkt_alloc_with_buffer(iface, sizeof(struct net_icmpv4_echo_req),
+					AF_INET, IPPROTO_ICMP, K_MSEC(100));
+	zassert_not_null(pkt, "NULL pkt");
+
+	if (net_ipv4_create(pkt, src4, &dst4) != 0 ||
+	    net_icmpv4_create(pkt, NET_ICMPV6_ECHO_REQUEST, 0) != 0) {
+		net_pkt_unref(pkt);
+		zassert_true(false, "Failed to create ICMP packet");
+	}
+
+	net_pkt_cursor_init(pkt);
+	net_ipv4_finalize(pkt, IPPROTO_ICMP);
+
+	if (net_send_data(pkt) != 0) {
+		net_pkt_unref(pkt);
+		zassert_true(false, "Failed to send packet");
+	}
+
+	ret = k_sem_take(&test_req_sem, K_MSEC(100));
+	if (ret != -EAGAIN) {
+		(void)net_icmp_cleanup_ctx(&ctx);
+		zassert_true(false, "ICMP request shouldn't be processed");
+	}
+
+	ret = net_icmp_cleanup_ctx(&ctx);
+	zassert_equal(ret, 0, "Cannot cleanup ICMP (%d)", ret);
+}
+
+ZTEST(icmp_tests, test_malformed_icmpv4_echo_request_on_ipv6)
+{
+	struct in6_addr dst6 = { 0 };
+	const struct in6_addr *src6;
+	struct net_icmp_ctx ctx;
+	struct net_if *iface;
+	struct net_pkt *pkt;
+	int ret;
+
+	k_sem_reset(&test_req_sem);
+
+	ret = net_icmp_init_ctx(&ctx, AF_INET, NET_ICMPV4_ECHO_REQUEST, 0,
+				icmp_request_handler);
+	zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
+
+	memcpy(&dst6, &recv_addr_6, sizeof(recv_addr_6));
+
+	/* Prepare malformed NET_ICMPV4_ECHO_REQUEST on IPv6 packet */
+	iface = net_if_ipv6_select_src_iface(&dst6);
+	zassert_not_null(iface, "NULL iface");
+
+	src6 = net_if_ipv6_select_src_addr(iface, &dst6);
+	zassert_not_null(src6, "NULL addr");
+
+	pkt = net_pkt_alloc_with_buffer(iface, sizeof(struct net_icmpv6_echo_req),
+					AF_INET6, IPPROTO_ICMPV6, K_MSEC(100));
+	zassert_not_null(pkt, "NULL pkt");
+
+	if (net_ipv6_create(pkt, src6, &dst6) != 0 ||
+	    net_icmpv6_create(pkt, NET_ICMPV4_ECHO_REQUEST, 0) != 0) {
+		net_pkt_unref(pkt);
+		zassert_true(false, "Failed to create ICMP packet");
+	}
+
+	net_pkt_cursor_init(pkt);
+	net_ipv6_finalize(pkt, IPPROTO_ICMPV6);
+
+	if (net_send_data(pkt) != 0) {
+		net_pkt_unref(pkt);
+		zassert_true(false, "Failed to send packet");
+	}
+
+	ret = k_sem_take(&test_req_sem, K_MSEC(100));
+	if (ret != -EAGAIN) {
+		(void)net_icmp_cleanup_ctx(&ctx);
+		zassert_true(false, "ICMP request shouldn't be processed");
+	}
+
+	ret = net_icmp_cleanup_ctx(&ctx);
+	zassert_equal(ret, 0, "Cannot cleanup ICMP (%d)", ret);
+}
+#endif /* defined(CONFIG_NET_IPV4) && defined(CONFIG_NET_IPV6) */
+
 static void *setup(void)
 {
 	if (IS_ENABLED(CONFIG_NET_TC_THREAD_COOPERATIVE)) {

tests/net/icmpv4/src/main.c

@@ -464,7 +464,7 @@ static void icmpv4_send_echo_rep(void)
 	struct net_pkt *pkt;
 	int ret;
 
-	ret = net_icmp_init_ctx(&ctx, NET_ICMPV4_ECHO_REPLY,
+	ret = net_icmp_init_ctx(&ctx, AF_INET, NET_ICMPV4_ECHO_REPLY,
 				0, handle_reply_msg);
 	zassert_equal(ret, 0, "Cannot register %s handler (%d)",
 		      STRINGIFY(NET_ICMPV4_ECHO_REPLY), ret);