From ac870330d220f6a961d193df1df483e4fc448a2c 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 --- 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 25c2ea05ef6a44..ef215e9287b315 100644 --- a/subsys/net/lib/dns/resolve.c +++ b/subsys/net/lib/dns/resolve.c @@ -525,6 +525,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 b799bdc3e81fa706e0ee8088f19792bc35a7a143 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 --- 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 52d8c154bc6110..460def50066de1 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); } static void test_dns_id_len(void) From eedd1bf03351c8bd012552b0a0e7297ef33cb910 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 --- 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 c5794510ef9d59..846078077d1185 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 f39e902b9b64729a766bede430b6085bab5cc951 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 --- 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 846078077d1185..90056bc9970993 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 84936ba0b9fd89c554927b004d7076303bd37315 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 --- tests/net/lib/dns_packet/src/main.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/net/lib/dns_packet/src/main.c b/tests/net/lib/dns_packet/src/main.c index 460def50066de1..797ed71043feee 100644 --- a/tests/net/lib/dns_packet/src/main.c +++ b/tests/net/lib/dns_packet/src/main.c @@ -50,6 +50,13 @@ static uint8_t query_mdns[] = { static uint16_t tid1 = 0xda0f; +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, +}; + + static int eval_query(const char *dname, uint16_t tid, enum dns_rr_type type, uint8_t *expected, uint16_t expected_len) { @@ -1260,6 +1267,21 @@ static void test_dns_flags_len(void) "DNS message length check failed (%d)", ret); } +static void test_dns_invalid_answer(void) +{ + 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); +} + void test_main(void) { ztest_test_suite(dns_tests, @@ -1271,7 +1293,8 @@ void test_main(void) ztest_unit_test(test_dns_id_len), ztest_unit_test(test_dns_flags_len), ztest_unit_test(test_dns_malformed_responses), - ztest_unit_test(test_dns_valid_responses) + ztest_unit_test(test_dns_valid_responses), + ztest_unit_test(test_dns_invalid_answer) ); ztest_run_test_suite(dns_tests);