public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v3 10/11] icmp: Validate packets received on ping sockets
Date: Tue, 16 Jan 2024 16:16:17 +1100	[thread overview]
Message-ID: <20240116051618.2746103-11-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240116051618.2746103-1-david@gibson.dropbear.id.au>

We access fields of packets received from ping sockets assuming they're
echo replies, without actually checking that.  Of course, we don't expect
anything else from the kernel, but it's probably best to verify.

While we're at it, also check for short packets, or a receive address of
the wrong family.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 icmp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/icmp.c b/icmp.c
index 79f6c8c..a9dc436 100644
--- a/icmp.c
+++ b/icmp.c
@@ -86,16 +86,25 @@ void icmp_sock_handler(const struct ctx *c, int af, union epoll_ref ref)
 		     pname, strerror(errno));
 		return;
 	}
+	if (sr.sa.sa_family != af)
+		goto unexpected;
 
 	if (af == AF_INET) {
 		struct icmphdr *ih4 = (struct icmphdr *)buf;
 
+		if ((size_t)n < sizeof(*ih4) || ih4->type != ICMP_ECHOREPLY)
+			goto unexpected;
+
 		/* Adjust packet back to guest-side ID */
 		ih4->un.echo.id = htons(ref.icmp.id);
 		seq = ntohs(ih4->un.echo.sequence);
 	} else if (af == AF_INET6) {
 		struct icmp6hdr *ih6 = (struct icmp6hdr *)buf;
 
+		if ((size_t)n < sizeof(*ih6) ||
+		    ih6->icmp6_type != ICMPV6_ECHO_REPLY)
+			goto unexpected;
+
 		/* Adjust packet back to guest-side ID */
 		ih6->icmp6_identifier = htons(ref.icmp.id);
 		seq = ntohs(ih6->icmp6_sequence);
@@ -118,6 +127,10 @@ void icmp_sock_handler(const struct ctx *c, int af, union epoll_ref ref)
 	else if (af == AF_INET6)
 		tap_icmp6_send(c, &sr.sa6.sin6_addr,
 			       tap_ip6_daddr(c, &sr.sa6.sin6_addr), buf, n);
+	return;
+
+unexpected:
+	warn("%s: Unexpected packet on ping socket", pname);
 }
 
 /**
-- 
@@ -86,16 +86,25 @@ void icmp_sock_handler(const struct ctx *c, int af, union epoll_ref ref)
 		     pname, strerror(errno));
 		return;
 	}
+	if (sr.sa.sa_family != af)
+		goto unexpected;
 
 	if (af == AF_INET) {
 		struct icmphdr *ih4 = (struct icmphdr *)buf;
 
+		if ((size_t)n < sizeof(*ih4) || ih4->type != ICMP_ECHOREPLY)
+			goto unexpected;
+
 		/* Adjust packet back to guest-side ID */
 		ih4->un.echo.id = htons(ref.icmp.id);
 		seq = ntohs(ih4->un.echo.sequence);
 	} else if (af == AF_INET6) {
 		struct icmp6hdr *ih6 = (struct icmp6hdr *)buf;
 
+		if ((size_t)n < sizeof(*ih6) ||
+		    ih6->icmp6_type != ICMPV6_ECHO_REPLY)
+			goto unexpected;
+
 		/* Adjust packet back to guest-side ID */
 		ih6->icmp6_identifier = htons(ref.icmp.id);
 		seq = ntohs(ih6->icmp6_sequence);
@@ -118,6 +127,10 @@ void icmp_sock_handler(const struct ctx *c, int af, union epoll_ref ref)
 	else if (af == AF_INET6)
 		tap_icmp6_send(c, &sr.sa6.sin6_addr,
 			       tap_ip6_daddr(c, &sr.sa6.sin6_addr), buf, n);
+	return;
+
+unexpected:
+	warn("%s: Unexpected packet on ping socket", pname);
 }
 
 /**
-- 
2.43.0


  parent reply	other threads:[~2024-01-16  5:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-16  5:16 [PATCH v3 00/11] RFC: ICMP reworks preliminary to flow table integration David Gibson
2024-01-16  5:16 ` [PATCH v3 01/11] icmp: Don't set "port" on destination sockaddr for ping sockets David Gibson
2024-01-16  5:16 ` [PATCH v3 02/11] icmp: Remove redundant initialisation of sendto() address David Gibson
2024-01-16  5:16 ` [PATCH v3 03/11] icmp: Don't attempt to handle "wrong direction" ping socket traffic David Gibson
2024-01-16  5:16 ` [PATCH v3 04/11] icmp: Don't attempt to match host IDs to guest IDs David Gibson
2024-01-16  5:16 ` [PATCH v3 05/11] icmp: Use -1 to represent "missing" sockets David Gibson
2024-01-16  5:16 ` [PATCH v3 06/11] icmp: Simplify socket expiry scanning David Gibson
2024-01-16  5:16 ` [PATCH v3 07/11] icmp: Share more between IPv4 and IPv6 paths in icmp_tap_handler() David Gibson
2024-01-16  5:16 ` [PATCH v3 08/11] icmp: Consolidate icmp_sock_handler() with icmpv6_sock_handler() David Gibson
2024-01-16  5:16 ` [PATCH v3 09/11] icmp: Warn on receive errors from ping sockets David Gibson
2024-01-16  5:16 ` David Gibson [this message]
2024-01-16  5:16 ` [PATCH v3 11/11] icmp: Dedicated functions for starting and closing ping sequences David Gibson
2024-01-23  0:39 ` [PATCH v3 00/11] RFC: ICMP reworks preliminary to flow table integration Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240116051618.2746103-11-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).