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 <bed...@gmail.com>
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 <bed...@gmail.com>
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
AAAA) 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
