Skip to content

Commit

Permalink
Merge pull request #20857 from benpicco/dns_msg-fix_skip
Browse files Browse the repository at this point in the history 
dns_msg: skip RDLENGTH_LENGTH field when skipping record
  • Loading branch information
@benpicco
benpicco authored Sep 27, 2024
2 parents 055c1e6 + 0463b27 commit 9bdb697
Show file tree
Hide file tree
Showing 6 changed files with 256 additions and 27 deletions.
37 changes: 24 additions & 13 deletions sys/net/application_layer/dns/msg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@

#include "net/dns/msg.h"

#define ENABLE_DEBUG 0
#include "debug.h"

static ssize_t _enc_domain_name(uint8_t *out, const char *domain_name)
{
/*
Expand Down Expand Up @@ -76,10 +79,13 @@ static ssize_t _skip_hostname(const uint8_t *buf, size_t len,

if (bufpos >= buflim) {
/* out-of-bound */
DEBUG("dns_msg: bufpos is out of bounds\n");
return -EBADMSG;
}

/* handle DNS Message Compression */
if (*bufpos >= 192) {
if (*bufpos & 0xc0) {
DEBUG("dns_msg: hostname is compressed\n");
if ((bufpos + 2) >= buflim) {
return -EBADMSG;
}
Expand All @@ -90,6 +96,7 @@ static ssize_t _skip_hostname(const uint8_t *buf, size_t len,
res += bufpos[res] + 1;
if ((&bufpos[res]) >= buflim) {
/* out-of-bound */
DEBUG("dns_msg: hostname out-of-bounds\n");
return -EBADMSG;
}
}
Expand Down Expand Up @@ -156,6 +163,7 @@ int dns_msg_parse_reply(const uint8_t *buf, size_t len, int family,
bufpos += tmp;
if ((bufpos + RR_TYPE_LENGTH + RR_CLASS_LENGTH +
RR_TTL_LENGTH + sizeof(uint16_t)) >= buflim) {
DEBUG("dns_msg: record beyond buf limit");
return -EBADMSG;
}
uint16_t _type = ntohs(_get_short(bufpos));
Expand All @@ -167,35 +175,38 @@ int dns_msg_parse_reply(const uint8_t *buf, size_t len, int family,
}
bufpos += RR_TTL_LENGTH;

unsigned addrlen = ntohs(_get_short(bufpos));
unsigned rdlen = ntohs(_get_short(bufpos));
bufpos += RR_RDLENGTH_LENGTH;
if ((bufpos + rdlen) > buflim) {
return -EBADMSG;
}

DEBUG("dns_msg: type: %u, class: %u, len: %u\n", _type, class, rdlen);

/* skip unwanted answers */
if ((class != DNS_CLASS_IN) ||
((_type == DNS_TYPE_A) && (family == AF_INET6)) ||
((_type == DNS_TYPE_AAAA) && (family == AF_INET)) ||
! ((_type == DNS_TYPE_A) || ((_type == DNS_TYPE_AAAA))
)) {
if (addrlen > len) {
if (rdlen > len) {
/* buffer wraps around memory space */
return -EBADMSG;
}
bufpos += addrlen;
bufpos += rdlen;
/* other out-of-bound is checked in `_skip_hostname()` at start of
* loop */
continue;
}
if (((addrlen != INADDRSZ) && (family == AF_INET)) ||
((addrlen != IN6ADDRSZ) && (family == AF_INET6)) ||
((addrlen != IN6ADDRSZ) && (addrlen != INADDRSZ) &&
if (((rdlen != INADDRSZ) && (family == AF_INET)) ||
((rdlen != IN6ADDRSZ) && (family == AF_INET6)) ||
((rdlen != IN6ADDRSZ) && (rdlen != INADDRSZ) &&
(family == AF_UNSPEC))) {
return -EBADMSG;
}
bufpos += RR_RDLENGTH_LENGTH;
if ((bufpos + addrlen) > buflim) {
return -EBADMSG;
}

memcpy(addr_out, bufpos, addrlen);
return addrlen;
memcpy(addr_out, bufpos, rdlen);
return rdlen;
}

return -EBADMSG;
Expand Down
37 changes: 23 additions & 14 deletions sys/net/application_layer/sock_dns/dns.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@
#include <string.h>

#include <arpa/inet.h>
#include <string.h>

#include "net/dns.h"
#include "net/dns/cache.h"
#include "net/dns/msg.h"
#include "net/sock/udp.h"
#include "net/sock/dns.h"

#define ENABLE_DEBUG 0
#include "debug.h"

/* min domain name length is 1, so minimum record length is 7 */
#define DNS_MIN_REPLY_LEN (unsigned)(sizeof(dns_hdr_t) + 7)

Expand Down Expand Up @@ -87,7 +91,7 @@ int sock_dns_query(const char *domain_name, void *addr_out, int family)

res = sock_udp_create(&sock_dns, NULL, &sock_dns_server, 0);
if (res) {
goto out;
return res;
}

uint16_t id = 0;
Expand All @@ -96,25 +100,30 @@ int sock_dns_query(const char *domain_name, void *addr_out, int family)

res = sock_udp_send(&sock_dns, dns_buf, buflen, NULL);
if (res <= 0) {
DEBUG("sock_dns: can't send: %s\n", strerror(-res));
continue;
}
res = sock_udp_recv(&sock_dns, dns_buf, sizeof(dns_buf), 1000000LU, NULL);
if (res > 0) {
if (res > (int)DNS_MIN_REPLY_LEN) {
uint32_t ttl;
if ((res = dns_msg_parse_reply(dns_buf, res, family,
addr_out, &ttl)) > 0) {
dns_cache_add(domain_name, addr_out, res, ttl);
goto out;
}
}
else {
res = -EBADMSG;
}
if (res < 0) {
DEBUG("sock_dns: can't receive: %s\n", strerror(-res));
continue;
}
if (res < (int)DNS_MIN_REPLY_LEN) {
DEBUG("sock_dns: reply too small (%d byte)\n", (int)res);
res = -EBADMSG;
continue;
}

uint32_t ttl;
if ((res = dns_msg_parse_reply(dns_buf, res, family,
addr_out, &ttl)) > 0) {
dns_cache_add(domain_name, addr_out, res, ttl);
break;
} else {
DEBUG("sock_dns: can't parse response\n");
}
}

out:
sock_udp_close(&sock_dns);
return res;
}
1 change: 1 addition & 0 deletions tests/unittests/tests-dns_msg/Makefile
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include $(RIOTBASE)/Makefile.base
5 changes: 5 additions & 0 deletions tests/unittests/tests-dns_msg/Makefile.include
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
USEMODULE += dns_msg

USEMODULE += gnrc_ipv6
USEMODULE += sock_udp
USEMODULE += posix_inet
159 changes: 159 additions & 0 deletions tests/unittests/tests-dns_msg/tests-dns_msg.c
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/*
* Copyright (C) 2024 ML!PA Consulting GmbH
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

#include <stdint.h>
#include <string.h>
#include "net/af.h"
#include "net/ipv6.h"
#include "ztimer.h"

#include "net/dns/msg.h"

#include "tests-dns_msg.h"

static void test_dns_msg_valid_AAAA(void)
{
const uint8_t dns_msg[] = {
/* in scapy notation:
* <DNS id=0 qr=1 opcode=QUERY aa=0 tc=0 rd=1 ra=1 z=0 ad=0 cd=0 rcode=ok
* qdcount=1 ancount=1 nscount=0 arcount=0
* qd=<DNSQR qname='example.org.' qtype=AAAA qclass=IN |>
* an=<DNSRR rrname='\\xc0\x0c' type=AAAA rclass=IN ttl=300
* rdata=2001:db8:4005:80b::200e |>
* ns=None ar=None |> */
0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x01,
0x00, 0x00, 0x00, 0x00, 0x07, 0x65, 0x78, 0x61,
0x6d, 0x70, 0x6c, 0x65, 0x03, 0x6f, 0x72, 0x67,
0x00, 0x00, 0x1c, 0x00, 0x01, 0xc0, 0x0c, 0x00,
0x1c, 0x00, 0x01, 0x00, 0x00, 0x01, 0x2c, 0x00,
0x10, 0x20, 0x01, 0x0d, 0xb8, 0x40, 0x05, 0x08,
0x0b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
0x0e
};
const uint8_t addr[] = {
/* 2001:db8:4005:80b::200e */
0x20, 0x01, 0x0d, 0xb8, 0x40, 0x05, 0x08, 0x0b,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x0e,
};
const uint32_t ttl = 300;

uint8_t addr_out[16];
uint32_t ttl_out;
int res = dns_msg_parse_reply(dns_msg, sizeof(dns_msg), AF_INET6, &addr_out, &ttl_out);
TEST_ASSERT_EQUAL_INT(16, res);
TEST_ASSERT_EQUAL_INT(ttl, ttl_out);
TEST_ASSERT_EQUAL_INT(0, memcmp(addr, addr_out, sizeof(addr)));
}

static void test_dns_msg_valid_dns64(void)
{
const uint8_t dns_msg[] = {
/* in scapy notation:
* <DNS id=0 qr=1 opcode=QUERY aa=0 tc=0 rd=1 ra=1 z=0 ad=0 cd=0 rcode=ok
qdcount=1 ancount=1 nscount=0 arcount=0
qd=<DNSQR qname='example.org.' qtype=AAAA qclass=IN |>
an=<DNSRR rrname='\\xc0\x0c' type=AAAA rclass=IN ttl=60
rdata=2001:db8:2b0:db32:0:1:8c52:7903 |>
ns=None ar=None |> */
0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x01,
0x00, 0x00, 0x00, 0x00, 0x07, 0x65, 0x78, 0x61,
0x6d, 0x70, 0x6c, 0x65, 0x03, 0x6f, 0x72, 0x67,
0x00, 0x00, 0x1c, 0x00, 0x01, 0xc0, 0x0c, 0x00,
0x1c, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00,
0x10, 0x20, 0x01, 0x0d, 0xb8, 0x02, 0xb0, 0xdb,
0x32, 0x00, 0x00, 0x00, 0x01, 0x8c, 0x52, 0x79,
0x03
};
const uint8_t addr[] = {
/* 2001:db8:2b0:db32:0:1:8c52:7903 */
0x20, 0x01, 0x0d, 0xb8, 0x02, 0xb0, 0xdb, 0x32,
0x00, 0x00, 0x00, 0x01, 0x8c, 0x52, 0x79, 0x03,
};
const uint32_t ttl = 60;

uint8_t addr_out[16];
uint32_t ttl_out;
int res = dns_msg_parse_reply(dns_msg, sizeof(dns_msg), AF_INET6, &addr_out, &ttl_out);
TEST_ASSERT_EQUAL_INT(16, res);
TEST_ASSERT_EQUAL_INT(ttl, ttl_out);
TEST_ASSERT_EQUAL_INT(0, memcmp(addr, addr_out, sizeof(addr)));
}

static void test_dns_msg_valid_dns64_w_long_cnames(void)
{
const uint8_t dns_msg[] = {
/* in scapy notation:
* <DNS id=0 qr=1 opcode=QUERY aa=0 tc=0 rd=1 ra=1 z=0 ad=0 cd=0 rcode=ok
* qdcount=1 ancount=3 nscount=0 arcount=0
* qd=<DNSQR qname='the.too.long.name.for.the.example.net.' qtype=AAAA qclass=IN |>
* an=<DNSRR rrname='\\xc0\x0c' type=CNAME rclass=IN ttl=600
* rdata='the.too.long.name.for.the.example.net.' |
* <DNSRR rrname='\\xc0C' type=CNAME rclass=IN ttl=90
* rdata='this-is-becoming-ridic.ulous.naming.cloud.example.com.' |
* <DNSRR rrname='\\xc0p' type=AAAA rclass=IN ttl=10
* rdata=2001:db8:2b0:db32:0:1:1432:418d |>>>
* ns=None ar=None |>
*/
0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x03,
0x00, 0x00, 0x00, 0x00, 0x03, 0x74, 0x68, 0x65,
0x03, 0x74, 0x6f, 0x6f, 0x04, 0x6c, 0x6f, 0x6e,
0x67, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x03, 0x66,
0x6f, 0x72, 0x03, 0x74, 0x68, 0x65, 0x07, 0x65,
0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x03, 0x6e,
0x65, 0x74, 0x00, 0x00, 0x1c, 0x00, 0x01, 0xc0,
0x0c, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x02,
0x58, 0x00, 0x21, 0x03, 0x74, 0x68, 0x65, 0x04,
0x65, 0x76, 0x65, 0x6e, 0x10, 0x6c, 0x6f, 0x6f,
0x6f, 0x6f, 0x6f, 0x6f, 0x6f, 0x6f, 0x6f, 0x6f,
0x6f, 0x6e, 0x67, 0x65, 0x72, 0x04, 0x6e, 0x61,
0x6d, 0x65, 0xc0, 0x26, 0xc0, 0x43, 0x00, 0x05,
0x00, 0x01, 0x00, 0x00, 0x00, 0x5a, 0x00, 0x37,
0x16, 0x74, 0x68, 0x69, 0x73, 0x2d, 0x69, 0x73,
0x2d, 0x62, 0x65, 0x63, 0x6f, 0x6d, 0x69, 0x6e,
0x67, 0x2d, 0x72, 0x69, 0x64, 0x69, 0x63, 0x05,
0x75, 0x6c, 0x6f, 0x75, 0x73, 0x06, 0x6e, 0x61,
0x6d, 0x69, 0x6e, 0x67, 0x05, 0x63, 0x6c, 0x6f,
0x75, 0x64, 0x07, 0x65, 0x78, 0x61, 0x6d, 0x70,
0x6c, 0x65, 0x03, 0x63, 0x6f, 0x6d, 0x00, 0xc0,
0x70, 0x00, 0x1c, 0x00, 0x01, 0x00, 0x00, 0x00,
0x0a, 0x00, 0x10, 0x20, 0x01, 0x0d, 0xb8, 0x02,
0xb0, 0xdb, 0x32, 0x00, 0x00, 0x00, 0x01, 0x14,
0x32, 0x41, 0x8d
};
const uint8_t addr[] = {
/* 2001:db8:2b0:db32:0:1:1432:418d */
0x20, 0x01, 0x0d, 0xb8, 0x02, 0xb0, 0xdb, 0x32,
0x00, 0x00, 0x00, 0x01, 0x14, 0x32, 0x41, 0x8d,
};
const uint32_t ttl = 10;

uint8_t addr_out[16];
uint32_t ttl_out;
int res = dns_msg_parse_reply(dns_msg, sizeof(dns_msg), AF_INET6, &addr_out, &ttl_out);
TEST_ASSERT_EQUAL_INT(16, res);
TEST_ASSERT_EQUAL_INT(ttl, ttl_out);
TEST_ASSERT_EQUAL_INT(0, memcmp(addr, addr_out, sizeof(addr)));
}

Test *tests_dns_msg_tests(void)
{
EMB_UNIT_TESTFIXTURES(fixtures) {
new_TestFixture(test_dns_msg_valid_AAAA),
new_TestFixture(test_dns_msg_valid_dns64),
new_TestFixture(test_dns_msg_valid_dns64_w_long_cnames),
};

EMB_UNIT_TESTCALLER(dns_msg_tests, NULL, NULL, fixtures);

return (Test *)&dns_msg_tests;
}

void tests_dns_msg(void)
{
TESTS_RUN(tests_dns_msg_tests());
}
44 changes: 44 additions & 0 deletions tests/unittests/tests-dns_msg/tests-dns_msg.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright (C) 2024 ML!PA Consulting GmbH
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @addtogroup unittests
* @{
*
* @file
* @brief Unittests for the `dns_msg` module
*
* @author Benjamin Valentin <benjamin.valentin@ml-pa.com>
*/
#ifndef TESTS_DNS_MSG_H
#define TESTS_DNS_MSG_H

#include "embUnit.h"

#ifdef __cplusplus
extern "C" {
#endif

/**
* @brief The entry point of this test suite.
*/
void tests_dns_msg(void);

/**
* @brief Generates tests for dns_msg
*
* @return embUnit tests if successful, NULL if not.
*/
Test *tests_dns_msg_tests(void);

#ifdef __cplusplus
}
#endif

#endif /* TESTS_DNS_MSG_H */
/** @} */

0 comments on commit 9bdb697

Please sign in to comment.