BUG/MEDIUM: dns: fix long loops in additional records parse on name failure

In resolv_validate_dns_response(), the additional records loop calls
resolv_read_name(). When it returns zero due to a bad response, the main
loop does a "continue" without making the "reader" pointer progress, so it
evaluates the exact same field again and again. Fortunately this is limited
by arcount which is 16 bits, but it means it can still iterate 65535 times
there, allocating and releasing an answer_record at each turn. Let's just
jump to the invalid_resp label that handles the cleaning. There was the
same pattern (without the allocation) with nscount a few lines above BTW.
These can possibly explain some situations where a high CPU usage observed
processing responses.

Seems like these were introduced in 2.2 with commit 37950c8d2
("BUG/MEDIUM: dns: improper parsing of aditional records")

This must be backported to stable versions.

diff --git a/src/resolvers.c b/src/resolvers.c
index 6556cc8a52e5e1928b5e2fe07b724b67592ffd4c..518421ecaaa8c749144abdc5f39bca911471ddae 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -1430,7 +1430,7 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe
                len = resolv_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE,
                                    &offset, 0);
                if (len == 0)
-                       continue;
+                       goto invalid_resp;
 
                if (reader + offset + 10 >= bufend)
                        goto invalid_resp;
@@ -1466,11 +1466,8 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe
                offset = 0;
                len = resolv_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0);
 
-               if (len == 0) {
-                       pool_free(resolv_answer_item_pool, answer_record);
-                       answer_record = NULL;
-                       continue;
-               }
+               if (len == 0)
+                       goto invalid_resp;
 
                memcpy(answer_record->name, tmpname, len);
                answer_record->name[len] = 0;