Skip to content

net: dns: Fix potential buffer overflow when unpacking labels #99683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 21, 2025
Merged

net: dns: Fix potential buffer overflow when unpacking labels #99683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ffac94
net: dns: Fix potential buffer overflow when unpacking labels
rlubos Nov 19, 2025
84aba3b
tests: net: dns_resolve: Add tests for dns_unpack_name()
rlubos Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions subsys/net/lib/dns/dns_pack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,6 @@ int mdns_unpack_query_header(struct dns_msg_t *msg, uint16_t *src_id)
int dns_unpack_name(const uint8_t *msg, int maxlen, const uint8_t *src,
struct net_buf *buf, const uint8_t **eol)
{
int dest_size = net_buf_tailroom(buf);
const uint8_t *end_of_label = NULL;
const uint8_t *curr_src = src;
int loop_check = 0, len = -1;
Expand Down Expand Up @@ -525,6 +524,8 @@ int dns_unpack_name(const uint8_t *msg, int maxlen, const uint8_t *src,
return -EMSGSIZE;
}
} else {
size_t dest_size = net_buf_tailroom(buf);

/* Max label length is 64 bytes (because 2 bits are
* used for pointer)
*/
Expand All @@ -533,8 +534,7 @@ int dns_unpack_name(const uint8_t *msg, int maxlen, const uint8_t *src,
return -EMSGSIZE;
}

if (((buf->data + label_len + 1) >=
(buf->data + dest_size)) ||
if ((label_len + 1 >= dest_size) ||
((curr_src + label_len) >= (msg + maxlen))) {
return -EMSGSIZE;
}
Expand Down
140 changes: 140 additions & 0 deletions tests/net/lib/dns_resolve/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ LOG_MODULE_REGISTER(net_test, CONFIG_DNS_RESOLVER_LOG_LEVEL);

#define NET_LOG_ENABLED 1
#include "net_private.h"
#include "dns_pack.h"

#if defined(CONFIG_DNS_RESOLVER_LOG_LEVEL_DBG)
#define DBG(fmt, ...) printk(fmt, ##__VA_ARGS__)
Expand Down Expand Up @@ -909,4 +910,143 @@ ZTEST(dns_resolve, test_dns_localhost_resolve_ipv6)
0, "not loopback address");
}

NET_BUF_POOL_DEFINE(test_dns_qname_pool, 2, CONFIG_DNS_RESOLVER_MAX_QUERY_LEN,
0, NULL);

ZTEST(dns_resolve, test_dns_unpack_name)
{
/* NULL string terminator serves a role of a final zero-length label */
static const uint8_t *test_records[] = {
/* example.com */
"\007example\003com",
/* www.zephyrproject.org */
"\003www\015zephyrproject\003org",
/* These records should barely fit (fills up the buffer size limit). */
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\076very_long_record_that_has_a_length_of_62_bytes_xxxxxxxxxxxxxxx",
};
static const uint8_t *expected_names[] = {
"example.com",
"www.zephyrproject.org",
"very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx."
"very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx."
"very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx."
"very_long_record_that_has_a_length_of_62_bytes_xxxxxxxxxxxxxxx",

};
struct net_buf *result;
int ret;

zassert_equal(CONFIG_DNS_RESOLVER_MAX_QUERY_LEN, 255, "Invalid test configuration");

for (int i = 0; i < ARRAY_SIZE(test_records); i++) {
const uint8_t *end_of_label = NULL;

result = net_buf_alloc(&test_dns_qname_pool, K_NO_WAIT);
zassert_not_null(result, "Failed to allocate buffer");

/* +1 to include NULL as terminating label */
ret = dns_unpack_name(test_records[i], strlen(test_records[i]) + 1,
test_records[i], result, &end_of_label);
zassert_equal(ret, strlen(expected_names[i]),
"Error parsing records (%d)", ret);
zassert_str_equal(expected_names[i], result->data,
"Parsed wrong name (%s)", result->data);
zassert_equal_ptr(test_records[i] + strlen(test_records[i]) + 1,
end_of_label, "Wrong end of label");

net_buf_unref(result);
}
}

ZTEST(dns_resolve, test_dns_unpack_name_with_pointer)
{
static const uint8_t test_records[] = {
/* www.example.com followed by ftp.example.com with pointer */
"\003www\007example\003com\000\003ftp\300\004" /* Last two bytes are pointer */
};
static const uint8_t *expected_names[] = {
"www.example.com",
"ftp.example.com",
};
const size_t offset_2nd_rec = 17;
const uint8_t *end_of_label = NULL;
struct net_buf *result;
int ret;

/* First name */
result = net_buf_alloc(&test_dns_qname_pool, K_NO_WAIT);
zassert_not_null(result, "Failed to allocate buffer");

ret = dns_unpack_name(test_records, sizeof(test_records),
test_records, result, &end_of_label);
zassert_equal(ret, strlen(expected_names[0]),
"Error parsing records (%d)", ret);
zassert_str_equal(expected_names[0], result->data,
"Parsed wrong name (%s)", result->data);
zassert_equal_ptr(test_records + offset_2nd_rec, end_of_label,
"Wrong end of label");

net_buf_unref(result);

/* Second name with a pointer within */
end_of_label = NULL;

result = net_buf_alloc(&test_dns_qname_pool, K_NO_WAIT);
zassert_not_null(result, "Failed to allocate buffer");

ret = dns_unpack_name(test_records, sizeof(test_records),
test_records + offset_2nd_rec, result, &end_of_label);
zassert_equal(ret, strlen(expected_names[1]),
"Error parsing records (%d)", ret);
zassert_str_equal(expected_names[1], result->data,
"Parsed wrong name (%s)", result->data);
/* -1 as end_of_label should point to the last byte in the buffer (pointer offset) */
zassert_equal_ptr(test_records + sizeof(test_records) - 1, end_of_label,
"Wrong end of label");

net_buf_unref(result);
}

ZTEST(dns_resolve, test_dns_unpack_name_overflow)
{
static const uint8_t *test_records[] = {
/* 4 records 63 bytes (252 bytes) + 3 bytes for dot separators,
* no space left for NULL terminator.
*/
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx",
/* 4 records fit (251 bytes), 4 bytes for dot separators, 5th one-byte
* record won't fit.
*/
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\077very_long_record_that_has_a_length_of_63_bytes_xxxxxxxxxxxxxxxx"
"\076very_long_record_that_has_a_length_of_62_bytes_xxxxxxxxxxxxxxx"
"\001x",
/* Single 64 byte record, that's forbidden (max record len is 63). */
"\100very_long_record_that_has_a_length_of_64_bytes_that_is_incorrect",
};
struct net_buf *result;
int ret;

zassert_equal(CONFIG_DNS_RESOLVER_MAX_QUERY_LEN, 255, "Invalid test configuration");

for (int i = 0; i < ARRAY_SIZE(test_records); i++) {
result = net_buf_alloc(&test_dns_qname_pool, K_NO_WAIT);
zassert_not_null(result, "Failed to allocate buffer");

/* +1 to include NULL as terminating label */
ret = dns_unpack_name(test_records[i], strlen(test_records[i]) + 1,
test_records[i], result, NULL);
zassert_equal(ret, -EMSGSIZE, "Name parsing should've failed");

net_buf_unref(result);
}
}

ZTEST_SUITE(dns_resolve, NULL, test_init, NULL, NULL, NULL);
Loading