On Mon, Apr 15, 2013 at 12:51:06PM +0200, Tommie Gannert wrote:
> 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().
>
Wow, I should probably appear in public only with a paper bag on my
face, I wrote that code :-/
Very good catch, Patrick, thank you!
> ---
> 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-23