[PATCH] sunrpc: xdr_bytes/xdr_string need to free buffer on error [BZ #21461]

[fweimer at redhat dot com (Florian Weimer)]

2017-05-05  Florian Weimer  <fweimer@redhat.com>

	[BZ #21461]
	* 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..b998b58 100644
--- a/NEWS
+++ b/NEWS
@@ -62,6 +62,9 @@ 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.
+
 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
+   <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 --git a/sunrpc/xdr.c b/sunrpc/xdr.c
index bfabf33..857f7c8 100644
--- a/sunrpc/xdr.c
+++ b/sunrpc/xdr.c
@@ -620,14 +620,24 @@ 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 (sp != *cpp)
+	    /* *cpp was NULL, so this function allocated a new
+	       buffer.  */
+	    free (sp);
 	  return FALSE;
 	}
-      /* fall into ... */
+      *cpp = sp;
+      return TRUE;
 
     case XDR_ENCODE:
       return xdr_opaque (xdrs, sp, nodesize);
@@ -781,14 +791,27 @@ 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 (sp != *cpp)
+	    /* *cpp was NULL, so this function allocated a new
+	       buffer.  */
+	    free (sp);
+	  return FALSE;
+	}
+      *cpp = sp;
+      return TRUE;
 
     case XDR_ENCODE:
       return xdr_opaque (xdrs, sp, size);