From patchwork Wed Apr 27 14:43:07 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 11908 Received: (qmail 88459 invoked by alias); 27 Apr 2016 14:43:13 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 88352 invoked by uid 89); 27 Apr 2016 14:43:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=112425, dual, family, xxx X-HELO: mx1.redhat.com Subject: Re: [PATCH] nss_dns: Check address length before creating addrinfo result [BZ #19831] To: libc-alpha@sourceware.org References: <56F58A74.70103@redhat.com> From: Florian Weimer Message-ID: <5720CFFB.2090107@redhat.com> Date: Wed, 27 Apr 2016 16:43:07 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <56F58A74.70103@redhat.com> On 03/25/2016 07:59 PM, Florian Weimer wrote: > This also consolidates the behavior between single (A or AAAA) and > dual (A and AAAA in parallel) queries. Single queries checked > the record length against the QTYPE, not the RRTYPE. Here is what I checked in. Thanks, Florian 2016-04-27 Florian Weimer [BZ #19831] * resolv/nss_dns/dns-host.c (rrtype_to_rdata_length): New function. (getanswer_r): Check RDATA length against RRTYPE and QTYPE. (gaih_getanswer_slice): Check RDATA length against RRTYPE. diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c index fb1d21c..403a005 100644 --- a/resolv/nss_dns/dns-host.c +++ b/resolv/nss_dns/dns-host.c @@ -134,6 +134,22 @@ extern enum nss_status _nss_dns_gethostbyname3_r (const char *name, int af, char **canonp); hidden_proto (_nss_dns_gethostbyname3_r) +/* Return the expected RDATA length for an address record type (A or + AAAA). */ +static int +rrtype_to_rdata_length (int type) +{ + switch (type) + { + case T_A: + return INADDRSZ; + case T_AAAA: + return IN6ADDRSZ; + default: + return -1; + } +} + enum nss_status _nss_dns_gethostbyname3_r (const char *name, int af, struct hostent *result, char *buffer, size_t buflen, int *errnop, @@ -888,6 +904,15 @@ getanswer_r (const querybuf *answer, int anslen, const char *qname, int qtype, cp += n; continue; /* XXX - had_error++ ? */ } + + /* Stop parsing at a record whose length is incorrect. */ + if (n != rrtype_to_rdata_length (type)) + { + ++had_error; + break; + } + + /* Skip records of the wrong type. */ if (n != result->h_length) { cp += n; @@ -1124,25 +1149,25 @@ gaih_getanswer_slice (const querybuf *answer, int anslen, const char *qname, } continue; } -#if 1 - // We should not see any types other than those explicitly listed - // below. Some types sent by server seem missing, though. Just - // collect the data for now. - if (__glibc_unlikely (type != T_A && type != T_AAAA)) -#else - if (__builtin_expect (type == T_SIG, 0) - || __builtin_expect (type == T_KEY, 0) - || __builtin_expect (type == T_NXT, 0) - || __builtin_expect (type == T_PTR, 0) - || __builtin_expect (type == T_DNAME, 0)) -#endif + + /* Stop parsing if we encounter a record with incorrect RDATA + length. */ + if (type == T_A || type == T_AAAA) + { + if (n != rrtype_to_rdata_length (type)) + { + ++had_error; + continue; + } + } + else { + /* Skip unknown records. */ cp += n; continue; } - if (type != T_A && type != T_AAAA) - abort (); + assert (type == T_A || type == T_AAAA); if (*pat == NULL) { uintptr_t pad = (-(uintptr_t) buffer @@ -1176,12 +1201,6 @@ gaih_getanswer_slice (const querybuf *answer, int anslen, const char *qname, } (*pat)->family = type == T_A ? AF_INET : AF_INET6; - if (__builtin_expect ((type == T_A && n != INADDRSZ) - || (type == T_AAAA && n != IN6ADDRSZ), 0)) - { - ++had_error; - continue; - } memcpy ((*pat)->addr, cp, n); cp += n; (*pat)->scopeid = 0;