* [PATCH] elf: Compute correct array size in _dl_init_paths [BZ #22606]
@ 2017-12-14 12:55 Florian Weimer
2017-12-14 13:49 ` Dmitry V. Levin
0 siblings, 1 reply; 2+ messages in thread
From: Florian Weimer @ 2017-12-14 12:55 UTC (permalink / raw)
To: libc-alpha
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.
diff --git a/NEWS b/NEWS
index abbe561dcb..eef51b65a6 100644
--- a/NEWS
+++ b/NEWS
@@ -125,6 +125,11 @@ Security related changes:
instead of NULL. This was a regression introduced with the new malloc
thread cache in glibc 2.26. Reported by Iain Buclaw.
+ 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:
[The release manager will add the list generated by
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 10d859bd35..82c9f46050 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -37,6 +37,7 @@
#include <sysdep.h>
#include <stap-probe.h>
#include <libc-pointer-arith.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)
@@ -685,9 +686,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 (struct r_search_path_elem));
if (rtld_search_dirs.dirs[0] == NULL)
{
errstring = N_("cannot create cache for search path");
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] elf: Compute correct array size in _dl_init_paths [BZ #22606]
2017-12-14 12:55 [PATCH] elf: Compute correct array size in _dl_init_paths [BZ #22606] Florian Weimer
@ 2017-12-14 13:49 ` Dmitry V. Levin
0 siblings, 0 replies; 2+ messages in thread
From: Dmitry V. Levin @ 2017-12-14 13:49 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 2936 bytes --]
On Thu, Dec 14, 2017 at 01:55:50PM +0100, Florian Weimer wrote:
> 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.
>
> diff --git a/NEWS b/NEWS
> index abbe561dcb..eef51b65a6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -125,6 +125,11 @@ Security related changes:
> instead of NULL. This was a regression introduced with the new malloc
> thread cache in glibc 2.26. Reported by Iain Buclaw.
>
> + 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:
>
> [The release manager will add the list generated by
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 10d859bd35..82c9f46050 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -37,6 +37,7 @@
> #include <sysdep.h>
> #include <stap-probe.h>
> #include <libc-pointer-arith.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)
> @@ -685,9 +686,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 (struct r_search_path_elem));
> if (rtld_search_dirs.dirs[0] == NULL)
> {
> errstring = N_("cannot create cache for search path");
This is OK. My personal preference for that kind of code is
> + rtld_search_dirs.dirs[0] = malloc (nsystem_dirs_len * round_size
> + * sizeof (**rtld_search_dirs.dirs));
so I certainly wouldn't object if it was used instead.
--
ldv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-12-14 13:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-14 12:55 [PATCH] elf: Compute correct array size in _dl_init_paths [BZ #22606] Florian Weimer
2017-12-14 13:49 ` Dmitry V. Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).