Hi there,
A couple of patches here to cleanup and fix some bugs introduced
by 13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b.
Baptiste
From 801e4f1d7ad1f9858f4b646fc4badebab3b46715 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann
Date: Wed, 19 Feb 2020 00:53:26 +0100
Subject: [PATCH 1/2] CLEANUP: remove obsolete comments
This patch removes some old comments introduced by
13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b.
Those comments are related to issues already fixed.
---
src/dns.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/dns.c b/src/dns.c
index bbc4f4ac1..3e52e1731 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1030,7 +1030,6 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
/* now parsing additional records */
nb_saved_records = 0;
- //TODO: check with Dinko for DNS poisoning
for (i = 0; i < dns_p->header.arcount; i++) {
if (reader >= bufend)
return DNS_RESP_INVALID;
@@ -1202,7 +1201,6 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
continue;
tmp_record->ar_item = dns_answer_record;
}
- //TODO: there is a leak for now, since we don't clean up AR records
LIST_ADDQ(&dns_p->ar_list, &dns_answer_record->list);
}
--
2.17.1
From 9c5f4f464380a1f67c7d5d802d6c05c0086cebfe Mon Sep 17 00:00:00 2001
From: Baptiste Assmann
Date: Wed, 19 Feb 2020 01:08:51 +0100
Subject: [PATCH 2/2] BUG/MEDIUM: dns: improper parsing of aditional records
13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b introduced parsing of
Additionnal DNS response section to pick up IP address when available.
That said, this introduced a side effect for other query types (A and
) leading to consider those responses invalid when parsing the
Additional section.
This patch avoids this situation by ensuring the Additional section is
parsed only for SRV queries.
---
src/dns.c | 26 ++
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/src/dns.c b/src/dns.c
index 3e52e1731..953f9414c 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1028,7 +1028,9 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
/* Save the number of records we really own */
dns_p->header.ancount = nb_saved_records;
- /* now parsing additional records */
+ /* now parsing additional records for SRV queries only */
+ if (dns_query->type != DNS_RTYPE_SRV)
+ goto skip_parsing_additional_records;
nb_saved_records = 0;
for (i = 0; i < dns_p->header.arcount; i++) {
if (reader >= bufend)
@@ -1043,25 +1045,7 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
if (len == 0) {
pool_free(dns_answer_item_pool, dns_answer_record);
- return DNS_RESP_INVALID;
- }
-
- /* Check if the current record dname is valid. previous_dname
- * points either to queried dname or last CNAME target */
- if (dns_query->type != DNS_RTYPE_SRV && memcmp(previous_dname, tmpname, len) != 0) {
- pool_free(dns_answer_item_pool, dns_answer_record);
- if (i == 0) {
-/* First record, means a mismatch issue between
- * queried dname and dname found in the first
- * record */
-return DNS_RESP_INVALID;
- }
- else {
-/* If not the first record, this means we have a
- * CNAME resolution error */
-return DNS_RESP_CNAME_ERROR;
- }
-
+ continue;
}
memcpy(dns_answer_record->name, tmpname, len);
@@ -1206,6 +1190,8 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
}
} /* for i 0 to arcount */
+ skip_parsing_additional_records:
+
/* Save the number of records we really own */
dns_p->header.arcount = nb_saved_records;
--
2.17.1
