BUG/MINOR: resolvers: validate TXT RDATA before reading

maxml reported on the public HAProxy mailing list that the DNS response
validator reads the first TXT character-string length byte before checking
that the record contains any RDATA.

When a TXT answer has RDLENGTH zero and ends exactly at bufend, the generic
RDATA bounds check accepts the empty range. The TXT parser then reads
reader[0] with reader == bufend, before rejecting the record. Short-circuit
evaluation of the subsequent condition does not protect this earlier read.

The report includes a minimized 31-byte DNS response which triggers an
AddressSanitizer one-byte heap-buffer-overflow with an exactly sized
libFuzzer input. All 13 original AFL++ crash inputs reach the same read.

The production caller uses a DNS_MAX_MSG_SIZE + 1 stack buffer and parses
at most DNS_MAX_MSG_SIZE bytes, so this read remains within the allocation.
The loaded value is discarded when the zero-length check fails. No
production crash or information disclosure was demonstrated.

Check that RDATA contains at least one byte before reading its first
character-string length. A valid empty TXT string remains accepted: its
RDLENGTH is one and its character-string length byte is zero.

This was introduced by e418e828aa ("MINOR: resolvers: basic TXT record
implementation"). It should be backported to 3.4. Older stable branches
are not affected.

diff --git a/src/resolvers.c b/src/resolvers.c
index 8b78ee34117c17870a5c3bb7af5942776685811d..a124c66e3d475ab2de58dff95075ec067b33b28c 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -1372,9 +1372,13 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe
                                /* TXT: sequence of [1-byte length | string bytes] *.
                                 * Only the first string is kept, further data is ignored.
                                 */
-                               int slen = (unsigned char)reader[0];
+                               int slen;
 
-                               if (answer_record->data_len < 1 || 1 + slen > answer_record->data_len)
+                               if (answer_record->data_len < 1)
+                                       goto invalid_resp;
+
+                               slen = (unsigned char)reader[0];
+                               if (1 + slen > answer_record->data_len)
                                        goto invalid_resp;
                                offset = answer_record->data_len;
                                memcpy(answer_record->data.target, reader + 1, slen);