2013/4/15 Tommie Gannert <tommie_at_spotify.com>
> 2013/4/15 Patrick Valsecchi <pvalsecc_at_cisco.com>
>
>> The first sub-string is fine, but with the second sub-string, the code
>> in ares_parse_txt.c (c-ares version 1.9.1) will have a bad behavior. The
>> loop line 146 will just compute a total length of 4+255=259 and the loop
>> line 164 can have two possible outcomes: crash or possible information leak.
>>
>
> Sounds reasonable. Adding a check that the last label ends on the rr_len
> boundary makes a lot of sense.
>
Something like this:
Subject: [PATCH] Check that TXT labels end on RR boundary in
ares_parse_txt_reply().
---
ares_parse_txt_reply.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/ares_parse_txt_reply.c b/ares_parse_txt_reply.c
index dcc6473..106c3f8 100644
--- a/ares_parse_txt_reply.c
+++ b/ares_parse_txt_reply.c
@@ -144,6 +144,12 @@ ares_parse_txt_reply (const unsigned char *abuf, int
alen,
strptr += substr_len + 1;
}
+ if (strptr != (aptr + rr_len))
+ {
+ status = ARES_EBADRESP;
+ break;
+ }
+
/* Including null byte */
txt_curr->txt = malloc (txt_curr->length + 1);
if (txt_curr->txt == NULL)
--
Tommie
Received on 2013-04-15