From patchwork Mon May 8 08:15:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 20297 Received: (qmail 50570 invoked by alias); 8 May 2017 08:15:37 -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 49435 invoked by uid 89); 8 May 2017 08:15:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=lesser X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 8228A23E6DF Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=fweimer@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 8228A23E6DF Subject: Re: [PATCH] sunrpc: xdr_bytes/xdr_string need to free buffer on error [BZ #21461] To: libc-alpha@sourceware.org References: <20170505131828.16EBC402C427D@oldenburg.str.redhat.com> From: Florian Weimer Message-ID: <91e971f6-a9d6-f6d3-5202-5b6392e23c94@redhat.com> Date: Mon, 8 May 2017 10:15:18 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <20170505131828.16EBC402C427D@oldenburg.str.redhat.com> The attached patch adds the CVE name and uses a more obvious condition (I think) for the allocation check in xdr_bytes/xdr_string. Any comments? Thanks, Florian CVE-2017-8804: xdr_bytes/xdr_string need to free buffer on error [BZ #21461] 2017-05-08 Florian Weimer [BZ #21461] CVE-2017-8804 * sunrpc/xdr.c (xdr_bytes): Deallocate allocated buffer on error. (xdr_string): Likewise. * sunrpc/Makefile (tests): Add tst-xdrmem3. (tests-special): Add mtrace-tst-xdrmem3.out. (generated): Add mtrace-tst-xdrmem3.out, tst-xdrmem3.mtrace. (tst-xdrmem3-ENV): Set MALLOC_TRACE. (mtrace-tst-xdrmem3.out): Run mtrace. (tst-xdrmem3): Link against full libc. * sunrpc/tst-xdrmem3.c: New file. diff --git a/NEWS b/NEWS index 5558ca3..9960aea 100644 --- a/NEWS +++ b/NEWS @@ -62,6 +62,10 @@ Security related changes: * The DNS stub resolver limits the advertised UDP buffer size to 1200 bytes, to avoid fragmentation-based spoofing attacks. +* The xdr_bytes and xdr_string routines free the internally allocated + buffer if deserialization of the buffer contents fails for any reason. + (CVE-2017-8804) + The following bugs are resolved with this release: [The release manager will add the list generated by diff --git a/sunrpc/Makefile b/sunrpc/Makefile index a5177ff..7a9117e 100644 --- a/sunrpc/Makefile +++ b/sunrpc/Makefile @@ -95,9 +95,16 @@ others += rpcgen endif tests = tst-xdrmem tst-xdrmem2 test-rpcent tst-udp-error tst-udp-timeout \ - tst-udp-nonblocking + tst-udp-nonblocking tst-xdrmem3 xtests := tst-getmyaddr +tests-special += $(objpfx)mtrace-tst-xdrmem3.out +generated += mtrace-tst-xdrmem3.out tst-xdrmem3.mtrace +tst-xdrmem3-ENV = MALLOC_TRACE=$(objpfx)tst-xdrmem3.mtrace +$(objpfx)mtrace-tst-xdrmem3.out: $(objpfx)tst-xdrmem3.out + $(common-objpfx)malloc/mtrace $(objpfx)tst-xdrmem3.mtrace > $@; \ + $(evaluate-test) + ifeq ($(have-thread-library),yes) xtests += thrsvc tests += tst-svc_register tst-udp-garbage @@ -162,6 +169,7 @@ BUILD_CPPFLAGS += $(sunrpc-CPPFLAGS) $(objpfx)tst-getmyaddr: $(common-objpfx)linkobj/libc.so $(objpfx)tst-xdrmem: $(common-objpfx)linkobj/libc.so $(objpfx)tst-xdrmem2: $(common-objpfx)linkobj/libc.so +$(objpfx)tst-xdrmem3: $(common-objpfx)linkobj/libc.so $(objpfx)tst-udp-error: $(common-objpfx)linkobj/libc.so $(objpfx)tst-svc_register: \ $(common-objpfx)linkobj/libc.so $(shared-thread-library) diff --git a/sunrpc/tst-xdrmem3.c b/sunrpc/tst-xdrmem3.c new file mode 100644 index 0000000..b3c72ae --- /dev/null +++ b/sunrpc/tst-xdrmem3.c @@ -0,0 +1,83 @@ +/* Test xdr_bytes, xdr_string behavior on deserialization failure. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +static int +do_test (void) +{ + mtrace (); + + /* If do_own_buffer, allocate the buffer and pass it to the + deserialization routine. Otherwise the routine is requested to + allocate the buffer. */ + for (int do_own_buffer = 0; do_own_buffer < 2; ++do_own_buffer) + { + /* Length 16 MiB, but only 2 bytes of data in the packet. */ + unsigned char buf[] = "\x01\x00\x00\x00\xff"; + XDR xdrs; + char *result; + unsigned int result_len; + + /* Test xdr_bytes. */ + xdrmem_create (&xdrs, (char *) buf, sizeof (buf), XDR_DECODE); + result_len = 0; + if (do_own_buffer) + { + char *own_buffer = xmalloc (10); + result = own_buffer; + TEST_VERIFY (!xdr_bytes (&xdrs, &result, &result_len, 10)); + TEST_VERIFY (result == own_buffer); + free (own_buffer); + } + else + { + result = NULL; + TEST_VERIFY (!xdr_bytes (&xdrs, &result, &result_len, -1)); + TEST_VERIFY (result == NULL); + } + TEST_VERIFY (result_len == 16 * 1024 * 1024); + xdr_destroy (&xdrs); + + /* Test xdr_string. */ + xdrmem_create (&xdrs, (char *) buf, sizeof (buf), XDR_DECODE); + if (do_own_buffer) + { + char *own_buffer = xmalloc (10); + result = own_buffer; + TEST_VERIFY (!xdr_string (&xdrs, &result, 10)); + TEST_VERIFY (result == own_buffer); + free (own_buffer); + } + else + { + result = NULL; + TEST_VERIFY (!xdr_string (&xdrs, &result, -1)); + TEST_VERIFY (result == NULL); + } + xdr_destroy (&xdrs); + } + + return 0; +} + +#include + diff --git a/sunrpc/xdr.c b/sunrpc/xdr.c index bfabf33..3fa7218 100644 --- a/sunrpc/xdr.c +++ b/sunrpc/xdr.c @@ -620,14 +620,23 @@ xdr_bytes (XDR *xdrs, char **cpp, u_int *sizep, u_int maxsize) } if (sp == NULL) { - *cpp = sp = (char *) mem_alloc (nodesize); + sp = (char *) mem_alloc (nodesize); + if (sp == NULL) + { + (void) __fxprintf (NULL, "%s: %s", __func__, + _("out of memory\n")); + return FALSE; + } } - if (sp == NULL) + if (!xdr_opaque (xdrs, sp, nodesize)) { - (void) __fxprintf (NULL, "%s: %s", __func__, _("out of memory\n")); + if (*cpp == NULL) + /* Deallocate the buffer allocated by this function. */ + free (sp); return FALSE; } - /* fall into ... */ + *cpp = sp; + return TRUE; case XDR_ENCODE: return xdr_opaque (xdrs, sp, nodesize); @@ -781,14 +790,26 @@ xdr_string (XDR *xdrs, char **cpp, u_int maxsize) { case XDR_DECODE: if (sp == NULL) - *cpp = sp = (char *) mem_alloc (nodesize); - if (sp == NULL) { - (void) __fxprintf (NULL, "%s: %s", __func__, _("out of memory\n")); - return FALSE; + sp = (char *) mem_alloc (nodesize); + if (sp == NULL) + { + (void) __fxprintf (NULL, "%s: %s", __func__, + _("out of memory\n")); + return FALSE; + } } sp[size] = 0; - /* fall into ... */ + + if (!xdr_opaque (xdrs, sp, size)) + { + if (*cpp == NULL) + /* Deallocate the buffer allocated by this function. */ + free (sp); + return FALSE; + } + *cpp = sp; + return TRUE; case XDR_ENCODE: return xdr_opaque (xdrs, sp, size);