small memory leak in libc/inet/resolv.c

[Joakim Tjernlund]

>
> We've discovered a small memory leak in __dns_lookup() when the A record
> in the DNS answer is preceded by one or more CNAME records.
>
> This can be reproduced by performing a gethostbyname() lookup on
> "www.google.com" or "www.yahoo.com".  After each CNAME record,
> __dns_lookup() leaks by 256 bytes.
>
> =====================================
> The problem code is in this segment:
>
>         first_answer = 1;
>         for (j = 0; j < h.ancount; j++) {
>             i = __decode_answer(packet, pos, packet_len, &ma);
>
>             <...snipped code for brevity...>
>
>             if (first_answer) {
>                 ma.buf = a->buf;
>                 ma.buflen = a->buflen;
>                 ma.add_count = a->add_count;
>                 memcpy(a, &ma, sizeof(ma));
>                 if (a->atype != T_SIG && (NULL == a->buf || (type != T_A && type != T_AAAA)))
>                     break;
>                 if (a->atype != type)
>                     continue;
>                 a->add_count = h.ancount - j - 1;
>                 if ((a->rdlength + sizeof(struct in_addr*)) * a->add_count > a->buflen)
>                     break;
>                 a->add_count = 0;
>                 first_answer = 0;
>             } else {
> =====================================
>
> On the first time through the loop (j == 0) with the CNAME record,
> ma.dotted is alloc'ed using a strdup() in decode_answer(), and memcpy()
> copies it to a->dotted. The loop is continued at "if (a->atype != type)".
>
> On the second time through the loop (j == 1), ma.dotted is again
> alloc'ed in decode_answer().  Now we have two allocated segments, one
> pointed-to by ma.dotted, and the other pointed-to by a->dotted.
> But memcpy() will over-write the pointer stored at a->dotted.
>
> My proposed solution is to check a->dotted before the memcpy() and free()
> it if it is non-NULL. (See the attached patch.)
>
> I've tested my solution with a number of different public DNS servers
> resolving a variety of names, but we're using an older version of uClibc.
>
> This leak was introduced in April 2010 when a similar free() call was
> removed for the test case when a DNS server returns a lone CNAME record.
> (ipv6.google.com)

free can handle NULL so you can skip the
  if (a->dotted)
test.

 Jocke