From 0896b3be94f64726653895e1cb57f2d4ac6c25ca Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Tue, 26 Nov 2024 15:40:53 +0200 Subject: [PATCH 1/5] net: dns: Check parsing error properly for response If the packet parsing fails in dns_unpack_response_query(), then do not continue further but bail out early. Signed-off-by: Jukka Rissanen (cherry picked from commit eb2550a441dbfb62bf55d8b7095fd0db78c1f2d6) --- subsys/net/lib/dns/resolve.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/subsys/net/lib/dns/resolve.c b/subsys/net/lib/dns/resolve.c index ff17b6b6257fe8..040751e622c86b 100644 --- a/subsys/net/lib/dns/resolve.c +++ b/subsys/net/lib/dns/resolve.c @@ -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; From b5cf7e8110a1ed9bf0ae5acf5fd1baff6b3e9883 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Tue, 26 Nov 2024 15:47:46 +0200 Subject: [PATCH 2/5] tests: net: dns: Add checking of malformed packet Make sure we test malformed packet parsing. Signed-off-by: Jukka Rissanen (cherry picked from commit 6f96915a1436845b1772af3e1dfb7174d4f219b6) --- tests/net/lib/dns_packet/src/main.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/net/lib/dns_packet/src/main.c b/tests/net/lib/dns_packet/src/main.c index e48477d0b03da8..bdf55c611f3fc6 100644 --- a/tests/net/lib/dns_packet/src/main.c +++ b/tests/net/lib/dns_packet/src/main.c @@ -709,6 +709,24 @@ static uint8_t resp_truncated_response_ipv4_5[] = { 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, @@ -1093,8 +1111,13 @@ static void run_dns_malformed_response(const char *test_case, 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); + } ret = dns_validate_msg(&dns_ctx, &dns_msg, &dns_id, &query_idx, NULL, &query_hash); @@ -1198,6 +1221,7 @@ static void test_dns_malformed_responses(void) 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) From aa3a9b42487f3de2d24519810eb9386e9e28475c Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Tue, 26 Nov 2024 16:40:28 +0200 Subject: [PATCH 3/5] net: dns: Validate source buffer length properly Make sure that when copying the qname, the source buffer is large enough for the data. Signed-off-by: Jukka Rissanen (cherry picked from commit 43c2b9cfe89f29ce41d0677cfd9eb4db71636026) --- subsys/net/lib/dns/dns_pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subsys/net/lib/dns/dns_pack.c b/subsys/net/lib/dns/dns_pack.c index 34b3651a4f0729..86bee64e7676e2 100644 --- a/subsys/net/lib/dns/dns_pack.c +++ b/subsys/net/lib/dns/dns_pack.c @@ -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; } From 8045f5da6bc256ce4c160ccc4e08e744610bc5d1 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Tue, 26 Nov 2024 17:48:43 +0200 Subject: [PATCH 4/5] net: dns: Check DNS answer properly The dns_unpack_answer() did not check the length of the message properly which can cause out of bounds read. Signed-off-by: Jukka Rissanen (cherry picked from commit 6e7fcff579b9bbfa826319b9a166b25a24ab3173) --- subsys/net/lib/dns/dns_pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subsys/net/lib/dns/dns_pack.c b/subsys/net/lib/dns/dns_pack.c index 86bee64e7676e2..3607514abf3993 100644 --- a/subsys/net/lib/dns/dns_pack.c +++ b/subsys/net/lib/dns/dns_pack.c @@ -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; } From 3fe004d874c308ff29ff56ca1ee95b2756b8d31b Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Tue, 26 Nov 2024 17:50:02 +0200 Subject: [PATCH 5/5] tests: net: dns: Add test for invalid DNS answer parsing Make sure we catch invalid answer during parsing. Signed-off-by: Jukka Rissanen (cherry picked from commit 16669ec4d5f8e0abbfadd06c1c5ec5584005efcd) --- tests/net/lib/dns_packet/src/main.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/net/lib/dns_packet/src/main.c b/tests/net/lib/dns_packet/src/main.c index bdf55c611f3fc6..363bdf3d3b51b8 100644 --- a/tests/net/lib/dns_packet/src/main.c +++ b/tests/net/lib/dns_packet/src/main.c @@ -1266,6 +1266,27 @@ ZTEST(dns_packet, test_dns_flags_len) "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; + 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)