[PATCH] sunrpc: xdr_bytes/xdr_string need to free buffer on error [BZ #21461] From: Florian Weimer <fweimer at redhat dot com> To: libc-alpha at sourceware dot org Date: Mon, 8 May 2017 10:15:18 +0200 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 <fweimer@redhat.com> [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 -Nurp glibc-2.22.orig/sunrpc/Makefile glibc-2.22/sunrpc/Makefile --- glibc-2.22.orig/sunrpc/Makefile 2015-08-05 09:42:21.000000000 +0300 +++ glibc-2.22/sunrpc/Makefile 2017-06-24 16:25:07.943976935 +0300 @@ -96,9 +96,16 @@ rpcgen-objs = rpc_main.o rpc_hout.o rpc_ extra-objs = $(rpcgen-objs) $(addprefix cross-,$(rpcgen-objs)) others += rpcgen -tests = tst-xdrmem tst-xdrmem2 test-rpcent +tests = tst-xdrmem tst-xdrmem2 test-rpcent 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 endif @@ -153,6 +160,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)rpcgen: $(addprefix $(objpfx),$(rpcgen-objs)) diff -Nurp glibc-2.22.orig/sunrpc/tst-xdrmem3.c glibc-2.22/sunrpc/tst-xdrmem3.c --- glibc-2.22.orig/sunrpc/tst-xdrmem3.c 1970-01-01 02:00:00.000000000 +0200 +++ glibc-2.22/sunrpc/tst-xdrmem3.c 2017-06-24 16:20:53.474180282 +0300 @@ -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 + <http://www.gnu.org/licenses/>. */ + +#include <mcheck.h> +#include <rpc/rpc.h> +#include <support/check.h> +#include <support/support.h> + +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 <support/test-driver.c> + diff -Nurp glibc-2.22.orig/sunrpc/xdr.c glibc-2.22/sunrpc/xdr.c --- glibc-2.22.orig/sunrpc/xdr.c 2015-08-05 09:42:21.000000000 +0300 +++ glibc-2.22/sunrpc/xdr.c 2017-06-24 16:20:53.474180282 +0300 @@ -624,14 +624,23 @@ xdr_bytes (xdrs, cpp, sizep, 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); @@ -787,14 +796,26 @@ xdr_string (xdrs, cpp, 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);