From patchwork Wed May 4 12:52:08 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 618440 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3r0Hyz31HXz9t66 for ; Wed, 4 May 2016 22:53:31 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=naqqUduh; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; q=dns; s=default; b= rg0DeXBmtVNfKkGS5FyWQgx9EY+1O2DFCEzCdECSwr+qEGHr5Xr0dqmFO0CLjyDs HaZ9HfbbwMYppZOEA9Los8T2lKdCvLzC8K2pTTTC+U8Ah5zBj0acsvIFgbmV4rPZ aq3evKMwehJd6/MfhTBUTbPi8UgP/TawDkOX/NydaCU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; s=default; bh=3E8nU4 YaXp3mGt5DklLslHo3yf4=; b=naqqUduhWozXd5H6pQw7n0OG/IKdBSjej458Cv EcqwP6X1mFG3INsB7LGL6F5Adp/5QYyCyqSL/iimalRoDtgNaJAGEKlDv6U08nie 9/sFk+hRdcFLv3wGLHRYiGWb5htXBbvE/Omy1+WpxcAQrVmQpmiz9tcq8T/U6cxI bPOa0= Received: (qmail 36072 invoked by alias); 4 May 2016 12:52:25 -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 35998 invoked by uid 89); 4 May 2016 12:52:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.0 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=*source, Hx-languages-length:5304, __VA_ARGS__, __va_args__ X-HELO: mx1.redhat.com Date: Wed, 04 May 2016 14:52:08 +0200 To: libc-alpha@sourceware.org Subject: [PATCH COMMITTED] getnameinfo: Return EAI_OVERFLOW in more cases [BZ #19787] User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20160504125208.A3E754100A9ED@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) The AF_LOCAL and AF_INET/AF_INET6 non-numerci service conversion did not return EAI_OVERFLOW if the supplied buffer was too small, silently returning truncated data. In the AF_INET/AF_INET6 numeric cases, the snprintf return value checking was incorrect. 2016-05-04 Florian Weimer [BZ #19787] * inet/getnameinfo.c (check_sprintf_result): New function. (CHECKED_SNPRINTF): New macro. (gni_host_inet_numeric): Use CHECKED_SNPRINTF to write the scope to the host buffer. (gni_host_local): Use checked_copy to copy the host name. (gni_serv_inet): Use CHECKED_SNPRINTF to write the service name. (gni_serv_local): Use checked_copy to copy the service name. (getnameinfo): Remove unnecessary truncation of result buffers. diff --git a/inet/getnameinfo.c b/inet/getnameinfo.c index c8de163..283da55 100644 --- a/inet/getnameinfo.c +++ b/inet/getnameinfo.c @@ -187,6 +187,39 @@ nrl_domainname (void) return domain; }; +/* Copy a string to a destination buffer with length checking. Return + EAI_OVERFLOW if the buffer is not large enough, and 0 on + success. */ +static int +checked_copy (char *dest, size_t destlen, const char *source) +{ + size_t source_length = strlen (source); + if (source_length + 1 > destlen) + return EAI_OVERFLOW; + memcpy (dest, source, source_length + 1); + return 0; +} + +/* Helper function for CHECKED_SNPRINTF below. */ +static int +check_sprintf_result (int result, size_t destlen) +{ + if (result < 0) + return EAI_SYSTEM; + if ((size_t) result >= destlen) + /* If ret == destlen, there was no room for the terminating NUL + character. */ + return EAI_OVERFLOW; + return 0; +} + +/* Format a string in the destination buffer. Return 0 on success, + EAI_OVERFLOW in case the buffer is too small, or EAI_SYSTEM on any + other error. */ +#define CHECKED_SNPRINTF(dest, destlen, format, ...) \ + check_sprintf_result \ + (__snprintf (dest, destlen, format, __VA_ARGS__), destlen) + /* Convert host name, AF_INET/AF_INET6 case, name only. */ static int gni_host_inet_name (struct scratch_buffer *tmpbuf, @@ -312,41 +345,22 @@ gni_host_inet_numeric (struct scratch_buffer *tmpbuf, uint32_t scopeid = sin6p->sin6_scope_id; if (scopeid != 0) { - /* Buffer is >= IFNAMSIZ+1. */ - char scopebuf[IFNAMSIZ + 1]; - char *scopeptr; - int ni_numericscope = 0; - size_t real_hostlen = __strnlen (host, hostlen); - size_t scopelen = 0; - - scopebuf[0] = SCOPE_DELIMITER; - scopebuf[1] = '\0'; - scopeptr = &scopebuf[1]; + size_t used_hostlen = __strnlen (host, hostlen); + /* Location of the scope string in the host buffer. */ + char *scope_start = host + used_hostlen; + size_t scope_length = hostlen - used_hostlen; if (IN6_IS_ADDR_LINKLOCAL (&sin6p->sin6_addr) || IN6_IS_ADDR_MC_LINKLOCAL (&sin6p->sin6_addr)) { - if (if_indextoname (scopeid, scopeptr) == NULL) - ++ni_numericscope; - else - scopelen = strlen (scopebuf); + char scopebuf[IFNAMSIZ]; + if (if_indextoname (scopeid, scopebuf) != NULL) + return CHECKED_SNPRINTF + (scope_start, scope_length, + "%c%s", SCOPE_DELIMITER, scopebuf); } - else - ++ni_numericscope; - - if (ni_numericscope) - scopelen = 1 + __snprintf (scopeptr, - (scopebuf - + sizeof scopebuf - - scopeptr), - "%u", scopeid); - - if (real_hostlen + scopelen + 1 > hostlen) - /* Signal the buffer is too small. This is - what inet_ntop does. */ - return EAI_OVERFLOW; - else - memcpy (host + real_hostlen, scopebuf, scopelen + 1); + return CHECKED_SNPRINTF + (scope_start, scope_length, "%c%u", SCOPE_DELIMITER, scopeid); } } else @@ -385,23 +399,17 @@ gni_host_local (struct scratch_buffer *tmpbuf, const struct sockaddr *sa, socklen_t addrlen, char *host, socklen_t hostlen, int flags) { - if (!(flags & NI_NUMERICHOST)) { struct utsname utsname; - - if (!uname (&utsname)) - { - strncpy (host, utsname.nodename, hostlen); - return 0; - } + if (uname (&utsname) == 0) + return checked_copy (host, hostlen, utsname.nodename); } if (flags & NI_NAMEREQD) return EAI_NONAME; - strncpy (host, "localhost", hostlen); - return 0; + return checked_copy (host, hostlen, "localhost"); } /* Convert the host part of an AF_LOCAK socket address. */ @@ -455,15 +463,10 @@ gni_serv_inet (struct scratch_buffer *tmpbuf, break; } if (s) - { - strncpy (serv, s->s_name, servlen); - return 0; - } + return checked_copy (serv, servlen, s->s_name); /* Fall through to numeric conversion. */ } - if (__snprintf (serv, servlen, "%d", ntohs (sinp->sin_port)) + 1 > servlen) - return EAI_OVERFLOW; - return 0; + return CHECKED_SNPRINTF (serv, servlen, "%d", ntohs (sinp->sin_port)); } /* Convert service to string, AF_LOCAL variant. */ @@ -472,8 +475,8 @@ gni_serv_local (struct scratch_buffer *tmpbuf, const struct sockaddr *sa, socklen_t addrlen, char *serv, socklen_t servlen, int flags) { - strncpy (serv, ((const struct sockaddr_un *) sa)->sun_path, servlen); - return 0; + return checked_copy + (serv, servlen, ((const struct sockaddr_un *) sa)->sun_path); } /* Convert service to string, dispatching to the implementations @@ -554,10 +557,6 @@ getnameinfo (const struct sockaddr *sa, socklen_t addrlen, char *host, } } - if (host && (hostlen > 0)) - host[hostlen-1] = 0; - if (serv && (servlen > 0)) - serv[servlen-1] = 0; scratch_buffer_free (&tmpbuf); return 0; }