mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/2] dns: Protect against too long hostnames and malicious packets
@ 2026-04-02  8:25 Sascha Hauer
  2026-04-02  8:25 ` [PATCH 1/2] net: dns: fix packet buffer overflow from long hostnames Sascha Hauer
  2026-04-02  8:25 ` [PATCH 2/2] net: dns: fix OOB read in dns_recv query type check Sascha Hauer
  0 siblings, 2 replies; 3+ messages in thread
From: Sascha Hauer @ 2026-04-02  8:25 UTC (permalink / raw)
  To: BAREBOX; +Cc: Claude Opus 4.6 (1M context)

This fixes some problems in the DNS code with wrong/malicious incoming
data.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (2):
      net: dns: fix packet buffer overflow from long hostnames
      net: dns: fix OOB read in dns_recv query type check

 net/dns.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)
---
base-commit: 0933e8f2ebf0d91dfcf177a4e4292b02921a53f1
change-id: 20260402-net-dns-buffer-overflows-0b3a418dcdc2

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] net: dns: fix packet buffer overflow from long hostnames
  2026-04-02  8:25 [PATCH 0/2] dns: Protect against too long hostnames and malicious packets Sascha Hauer
@ 2026-04-02  8:25 ` Sascha Hauer
  2026-04-02  8:25 ` [PATCH 2/2] net: dns: fix OOB read in dns_recv query type check Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Sascha Hauer @ 2026-04-02  8:25 UTC (permalink / raw)
  To: BAREBOX; +Cc: Claude Opus 4.6 (1M context)

dns_send() uses strcpy() to copy the DNS-encoded hostname into the
packet buffer with no length check. The packet buffer has ~1482 bytes
available for the query name (PKTSIZE minus ethernet/IP/UDP headers,
DNS header, and query trailer). A hostname+domain combination longer
than this overflows the packet buffer.

The hostname comes from resolv() callers (TFTP server names, NFS
paths, user commands, etc.) and the domain from the user-configurable
global.net.domainname variable.

Fix by validating the encoded name length against the available packet
space before copying, and replace strcpy with memcpy using the known
length. Return -ENAMETOOLONG if the name doesn't fit.

Note the final component of the hostname is a dot ('.') which gets
replaced with a 0 in the "replace dots in fullname with chunk len" loop.
As we now calculate the strlen before that loop it will include that
final 0 already, so we must drop the "Mark end of host name" part.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 net/dns.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/dns.c b/net/dns.c
index 8fbd13cdc3..fde1439469 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -70,6 +70,10 @@ static int dns_send(const char *name)
 	unsigned char *packet = net_udp_get_payload(dns_con);
 	unsigned char *p, *s, *fullname, *dotptr;
 	const unsigned char *domain;
+	int namelen;
+	/* max UDP payload minus header and query trailer (5 bytes) */
+	const int maxlen = PKTSIZE - ETHER_HDR_SIZE - sizeof(struct iphdr) -
+			   sizeof(struct udphdr) - offsetof(struct header, data) - 5;
 
 	/* generate "difficult" to predict transaction id */
 	dns_req_id = dns_timer_start + (dns_timer_start >> 16);
@@ -90,6 +94,14 @@ static int dns_send(const char *name)
 	else
 		fullname = basprintf(".%s.", name);
 
+	namelen = strlen(fullname);
+	if (namelen > maxlen) {
+		pr_err("hostname too long for DNS query (%d > %d)\n",
+		       namelen, maxlen);
+		free(fullname);
+		return -ENAMETOOLONG;
+	}
+
 	/* replace dots in fullname with chunk len */
 	dotptr = fullname;
 	do {
@@ -104,11 +116,10 @@ static int dns_send(const char *name)
 	} while (*(dotptr + 1));
 	*dotptr = 0;
 
-	strcpy(header->data, fullname);
+	memcpy(header->data, fullname, namelen);
 
-	p = header->data + strlen(fullname);
+	p = header->data + namelen;
 
-	*p++ = 0;			/* Mark end of host name */
 	*p++ = 0;			/* Some servers require double null */
 	*p++ = (unsigned char)qtype;	/* Query Type */
 

-- 
2.47.3




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] net: dns: fix OOB read in dns_recv query type check
  2026-04-02  8:25 [PATCH 0/2] dns: Protect against too long hostnames and malicious packets Sascha Hauer
  2026-04-02  8:25 ` [PATCH 1/2] net: dns: fix packet buffer overflow from long hostnames Sascha Hauer
@ 2026-04-02  8:25 ` Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Sascha Hauer @ 2026-04-02  8:25 UTC (permalink / raw)
  To: BAREBOX; +Cc: Claude Opus 4.6 (1M context)

After skipping the query name in a DNS response, dns_recv() reads
p[1] and p[2] to check the query type BEFORE validating that these
offsets are within the packet bounds. The bounds check '&p[5] > e'
follows the reads, but by then the OOB access has already occurred.

If the query name's null terminator is at or near the end of the
packet, p[1] and p[2] read 1-2 bytes past the packet data.

Fix by moving the bounds check before the reads.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 net/dns.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/dns.c b/net/dns.c
index fde1439469..5a3ee30720 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -166,8 +166,13 @@ static void dns_recv(struct header *header, unsigned len)
 		continue;
 
 	/* We sent query class 1, query type 1 */
+	if (&p[5] > e) {
+		pr_debug("DNS response too short\n");
+		return;
+	}
+
 	tmp = p[1] | (p[2] << 8);
-	if (&p[5] > e || ntohs(tmp) != DNS_A_RECORD) {
+	if (ntohs(tmp) != DNS_A_RECORD) {
 		pr_debug("DNS response was not A record\n");
 		return;
 	}

-- 
2.47.3




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-02  8:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-02  8:25 [PATCH 0/2] dns: Protect against too long hostnames and malicious packets Sascha Hauer
2026-04-02  8:25 ` [PATCH 1/2] net: dns: fix packet buffer overflow from long hostnames Sascha Hauer
2026-04-02  8:25 ` [PATCH 2/2] net: dns: fix OOB read in dns_recv query type check Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox