Skip to content
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.

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

[Backport v4.0-branch] DNS parsing issues #82289

Merged
merged 5 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions subsys/net/lib/dns/dns_pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ int dns_unpack_answer(struct dns_msg_t *dns_msg, int dname_ptr, uint32_t *ttl,
*
* See RFC-1035 4.1.3. Resource record format
*/
rem_size = dns_msg->msg_size - dname_len;
rem_size = dns_msg->msg_size - dns_msg->answer_offset - dname_len;
if (rem_size < 2 + 2 + 4 + 2) {
return -EINVAL;
}
Expand Down Expand Up @@ -394,7 +394,7 @@ int dns_copy_qname(uint8_t *buf, uint16_t *len, uint16_t size,
/* validate that the label (i.e. size + elements),
* fits the current msg buffer
*/
if (DNS_LABEL_LEN_SIZE + lb_size > size - *len) {
if (DNS_LABEL_LEN_SIZE + lb_size > MIN(size - *len, msg_size - pos)) {
rc = -ENOMEM;
break;
}
Expand Down
5 changes: 5 additions & 0 deletions subsys/net/lib/dns/resolve.c
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,11 @@ int dns_validate_msg(struct dns_resolve_context *ctx,

ret = dns_unpack_response_query(dns_msg);
if (ret < 0) {
if (ret == -ENOMEM) {
ret = DNS_EAI_FAIL;
goto quit;
}

/* Check mDNS like above */
if (*dns_id > 0) {
ret = DNS_EAI_FAIL;
Expand Down
49 changes: 47 additions & 2 deletions tests/net/lib/dns_packet/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,24 @@
0x00, 0x04,
};

static uint8_t resp_truncated_response_ipv4_6[] = {
/* DNS msg header (12 bytes) */
/* Id (0) */
0x00, 0x00,
/* Flags (response, rcode = 1) */
0x80, 0x01,
/* Number of questions */
0x00, 0x01,
/* Number of answers */
0x00, 0x00,
/* Number of authority RRs */
0x00, 0x00,
/* Number of additional RRs */
0x00, 0x00,

/* Rest of the data is missing */
};

static uint8_t resp_valid_response_ipv4_6[] = {
/* DNS msg header (12 bytes) */
0xb0, 0x41, 0x81, 0x80, 0x00, 0x01, 0x00, 0x01,
Expand Down Expand Up @@ -1093,8 +1111,13 @@

dns_id = dns_unpack_header_id(dns_msg.msg);

setup_dns_context(&dns_ctx, 0, dns_id, query, sizeof(query),
DNS_QUERY_TYPE_A);
/* If the message is longer than 12 bytes, it could be a valid DNS message
* in which case setup the context for the reply.
*/
if (len > 12) {
setup_dns_context(&dns_ctx, 0, dns_id, query, sizeof(query),
DNS_QUERY_TYPE_A);
}

Check notice on line 1120 in tests/net/lib/dns_packet/src/main.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

tests/net/lib/dns_packet/src/main.c:1120 - setup_dns_context(&dns_ctx, 0, dns_id, query, sizeof(query), - DNS_QUERY_TYPE_A); + setup_dns_context(&dns_ctx, 0, dns_id, query, sizeof(query), DNS_QUERY_TYPE_A);

Check notice on line 1120 in tests/net/lib/dns_packet/src/main.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

tests/net/lib/dns_packet/src/main.c:1120 - setup_dns_context(&dns_ctx, 0, dns_id, query, sizeof(query), - DNS_QUERY_TYPE_A); + setup_dns_context(&dns_ctx, 0, dns_id, query, sizeof(query), DNS_QUERY_TYPE_A);

ret = dns_validate_msg(&dns_ctx, &dns_msg, &dns_id, &query_idx,
NULL, &query_hash);
Expand Down Expand Up @@ -1198,6 +1221,7 @@
RUN_MALFORMED_TEST(resp_truncated_response_ipv4_3);
RUN_MALFORMED_TEST(resp_truncated_response_ipv4_4);
RUN_MALFORMED_TEST(resp_truncated_response_ipv4_5);
RUN_MALFORMED_TEST(resp_truncated_response_ipv4_6);
}

ZTEST(dns_packet, test_dns_malformed_and_valid_responses)
Expand Down Expand Up @@ -1242,6 +1266,27 @@
"DNS message length check failed (%d)", ret);
}

static uint8_t invalid_answer_resp_ipv4[18] = {
/* DNS msg header (12 bytes) */
0x01, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x01, 0x00, 0x01,
};

ZTEST(dns_packet, test_dns_invalid_answer)
{
struct dns_msg_t dns_msg = { 0 };
enum dns_rr_type type;

Check notice on line 1278 in tests/net/lib/dns_packet/src/main.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

tests/net/lib/dns_packet/src/main.c:1278 - 0x01, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x01, 0x00, 0x01, + 0x01, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, }; ZTEST(dns_packet, test_dns_invalid_answer) { - struct dns_msg_t dns_msg = { 0 }; + struct dns_msg_t dns_msg = {0};

Check notice on line 1278 in tests/net/lib/dns_packet/src/main.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

tests/net/lib/dns_packet/src/main.c:1278 - 0x01, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x01, 0x00, 0x01, + 0x01, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, }; ZTEST(dns_packet, test_dns_invalid_answer) { - struct dns_msg_t dns_msg = { 0 }; + struct dns_msg_t dns_msg = {0};
uint32_t ttl;
int ret;

dns_msg.msg = invalid_answer_resp_ipv4;
dns_msg.msg_size = sizeof(invalid_answer_resp_ipv4);
dns_msg.answer_offset = 12;

ret = dns_unpack_answer(&dns_msg, 0, &ttl, &type);
zassert_equal(ret, -EINVAL, "DNS message answer check succeed (%d)", ret);
}

ZTEST_SUITE(dns_packet, NULL, NULL, NULL, NULL, NULL);
/* TODO:
* 1) add malformed DNS data (mostly done)
Expand Down
Loading