[COMMITTED 2.24 2/3] elf: Compute correct array size in _dl_init_paths [BZ #22606]

[COMMITTED 2.24 2/3] elf: Compute correct array size in _dl_init_paths [BZ #22606]

[Aurelien Jarno]

From: Florian Weimer <fweimer@redhat.com>

(cherry picked from commit 8a0b17e48b83e933960dfeb8fa08b259f03f310e)
---
 ChangeLog     |  8 ++++++++
 NEWS          |  5 +++++
 elf/dl-load.c | 14 +++++++-------
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6e648c310d..90e4444444 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2017-12-14  Florian Weimer  <fweimer@redhat.com>
+
+	[BZ #22606]
+	CVE-2017-1000408
+	* elf/dl-load.c (system_dirs): Update comment.
+	(nsystem_dirs_len): Use array_length.
+	(_dl_init_paths): Use nsystem_dirs_len to compute the array size.
+
 2017-11-02  Florian Weimer  <fweimer@redhat.com>
 
 	Add array_length and array_end macros.
diff --git a/NEWS b/NEWS
index bc32643255..9de14ffba0 100644
--- a/NEWS
+++ b/NEWS
@@ -34,6 +34,11 @@ Security related changes:
   without GLOB_NOESCAPE, could write past the end of a buffer while
   unescaping user names.  Reported by Tim Rühsen.
 
+* CVE-2017-1000408: Incorrect array size computation in _dl_init_paths leads
+  to the allocation of too much memory.  (This is not a security bug per se,
+  it is mentioned here only because of the CVE assignment.)  Reported by
+  Qualys.
+
 The following bugs are resolved with this release:
 
   [20790] Fix rpcgen buffer overrun
diff --git a/elf/dl-load.c b/elf/dl-load.c
index c0d6249373..0d46c16ea7 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -37,6 +37,7 @@
 #include <sysdep.h>
 #include <stap-probe.h>
 #include <libc-internal.h>
+#include <array_length.h>
 
 #include <dl-dst.h>
 #include <dl-load.h>
@@ -103,7 +104,9 @@ static size_t ncapstr attribute_relro;
 static size_t max_capstrlen attribute_relro;
 
 
-/* Get the generated information about the trusted directories.  */
+/* Get the generated information about the trusted directories.  Use
+   an array of concatenated strings to avoid relocations.  See
+   gen-trusted-dirs.awk.  */
 #include "trusted-dirs.h"
 
 static const char system_dirs[] = SYSTEM_DIRS;
@@ -111,9 +114,7 @@ static const size_t system_dirs_len[] =
 {
   SYSTEM_DIRS_LEN
 };
-#define nsystem_dirs_len \
-  (sizeof (system_dirs_len) / sizeof (system_dirs_len[0]))
-
+#define nsystem_dirs_len array_length (system_dirs_len)
 
 static bool
 is_trusted_path (const char *path, size_t len)
@@ -688,9 +689,8 @@ _dl_init_paths (const char *llp)
 		 + ncapstr * sizeof (enum r_dir_status))
 		/ sizeof (struct r_search_path_elem));
 
-  rtld_search_dirs.dirs[0] = (struct r_search_path_elem *)
-    malloc ((sizeof (system_dirs) / sizeof (system_dirs[0]))
-	    * round_size * sizeof (struct r_search_path_elem));
+  rtld_search_dirs.dirs[0] = malloc (nsystem_dirs_len * round_size
+				     * sizeof (*rtld_search_dirs.dirs[0]));
   if (rtld_search_dirs.dirs[0] == NULL)
     {
       errstring = N_("cannot create cache for search path");
-- 
2.15.1

[COMMITTED 2.24 3/3] elf: Count components of the expanded path in _dl_init_path [BZ #22607]

[Aurelien Jarno]

From: Florian Weimer <fweimer@redhat.com>

(cherry picked from commit 3ff3dfa5af313a6ea33f3393916f30eece4f0171)
---
 ChangeLog     |  7 +++++++
 NEWS          |  6 ++++++
 elf/dl-load.c | 13 ++++---------
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 90e4444444..2c2e9d5b19 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2017-12-14  Florian Weimer  <fweimer@redhat.com>
+
+	[BZ #22607]
+	CVE-2017-1000409
+	* elf/dl-load.c (_dl_init_paths): Compute number of components in
+	the expanded path string.
+
 2017-12-14  Florian Weimer  <fweimer@redhat.com>
 
 	[BZ #22606]
diff --git a/NEWS b/NEWS
index 9de14ffba0..9e20117a81 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,12 @@ Security related changes:
   it is mentioned here only because of the CVE assignment.)  Reported by
   Qualys.
 
+* CVE-2017-1000409: Buffer overflow in _dl_init_paths due to miscomputation
+  of the number of search path components.  (This is not a security
+  vulnerability per se because no trust boundary is crossed if the fix for
+  CVE-2017-1000366 has been applied, but it is mentioned here only because
+  of the CVE assignment.)  Reported by Qualys.
+
 The following bugs are resolved with this release:
 
   [20790] Fix rpcgen buffer overrun
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 0d46c16ea7..64f55145a2 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -776,8 +776,6 @@ _dl_init_paths (const char *llp)
 
   if (llp != NULL && *llp != '\0')
     {
-      size_t nllp;
-      const char *cp = llp;
       char *llp_tmp;
 
 #ifdef SHARED
@@ -800,13 +798,10 @@ _dl_init_paths (const char *llp)
 
       /* Decompose the LD_LIBRARY_PATH contents.  First determine how many
 	 elements it has.  */
-      nllp = 1;
-      while (*cp)
-	{
-	  if (*cp == ':' || *cp == ';')
-	    ++nllp;
-	  ++cp;
-	}
+      size_t nllp = 1;
+      for (const char *cp = llp_tmp; *cp != '\0'; ++cp)
+	if (*cp == ':' || *cp == ';')
+	  ++nllp;
 
       env_path_list.dirs = (struct r_search_path_elem **)
 	malloc ((nllp + 1) * sizeof (struct r_search_path_elem *));
-- 
2.15.1

[COMMITTED 2.24 1/3] <array_length.h>: New array_length and array_end macros

[Aurelien Jarno]

From: Florian Weimer <fweimer@redhat.com>

(cherry picked from commit c94a5688fb1228a862b2d4a3f1239cdc0e3349e5)
---
 ChangeLog              |  5 +++++
 include/array_length.h | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 include/array_length.h

diff --git a/ChangeLog b/ChangeLog
index 172df43e16..6e648c310d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2017-11-02  Florian Weimer  <fweimer@redhat.com>
+
+	Add array_length and array_end macros.
+	* include/array_length.h: New file.
+
 2017-11-02  Florian Weimer  <fweimer@redhat.com>
 
 	[BZ #22332]
diff --git a/include/array_length.h b/include/array_length.h
new file mode 100644
index 0000000000..cb4a8b2a56
--- /dev/null
+++ b/include/array_length.h
@@ -0,0 +1,36 @@
+/* The array_length and array_end macros.
+   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/>.  */
+
+#ifndef _ARRAY_LENGTH_H
+#define _ARRAY_LENGTH_H
+
+/* array_length (VAR) is the number of elements in the array VAR.  VAR
+   must evaluate to an array, not a pointer.  */
+#define array_length(var)                                               \
+  __extension__ ({                                                      \
+    _Static_assert (!__builtin_types_compatible_p                       \
+                    (__typeof (var), __typeof (&(var)[0])),             \
+                    "argument must be an array");                       \
+    sizeof (var) / sizeof ((var)[0]);                                   \
+  })
+
+/* array_end (VAR) is a pointer one past the end of the array VAR.
+   VAR must evaluate to an array, not a pointer.  */
+#define array_end(var) (&(var)[array_length (var)])
+
+#endif /* _ARRAY_LENGTH_H */
-- 
2.15.1