On 1/17/20 9:00 AM, Bruno Haible wrote:
Thank you very much for this report! I expect that the fix will also need
to go into glibc.
Yes, thanks, I filed a bug report here:
https://sourceware.org/bugzilla/show_bug.cgi?id=25414
It seems that your patch is incomplete, as there's a memory leak in the
unlikely case that the malloc call fails. Something like the attached
patch instead, perhaps?
>From 8d5f7d7e43b8830acde4f82b93cc7b970d9ec435 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Fri, 17 Jan 2020 11:51:26 -0800
Subject: [PATCH] glob: Fix use-after-free bug.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Reported by Tim Rühsen <tim.rueh...@gmx.de> in
<https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00102.html>.
* lib/glob.c (__glob): Delay freeing dirname until after the use of
end_name.
---
ChangeLog | 9 +++++++++
lib/glob.c | 21 +++++++++++++--------
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 643dba312..04b139a6a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2020-01-17 Bruno Haible <br...@clisp.org>
+ Paul Eggert <egg...@cs.ucla.edu>
+
+ glob: Fix use-after-free bug.
+ Reported by Tim Rühsen <tim.rueh...@gmx.de> in
+ <https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00102.html>.
+ * lib/glob.c (__glob): Delay freeing dirname until after the use of
+ end_name.
+
2020-01-16 Siddhesh Poyarekar <siddh...@gotplt.org>
vcs-to-changelog: Fix parsing of fndecl without args.
diff --git a/lib/glob.c b/lib/glob.c
index a67cbb67e..e0928312a 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -843,24 +843,27 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
size_t home_len = strlen (p->pw_dir);
size_t rest_len = end_name == NULL ? 0 : strlen (end_name);
+ /* dirname contains end_name; we can't free it now. */
+ char *prev_dirname =
+ (__glibc_unlikely (malloc_dirname) ? dirname : NULL);
char *d;
- if (__glibc_unlikely (malloc_dirname))
- free (dirname);
- malloc_dirname = 0;
-
if (glob_use_alloca (alloca_used, home_len + rest_len + 1))
- dirname = alloca_account (home_len + rest_len + 1,
- alloca_used);
+ {
+ dirname = alloca_account (home_len + rest_len + 1,
+ alloca_used);
+ malloc_dirname = 0;
+ }
else
{
- dirname = malloc (home_len + rest_len + 1);
- if (dirname == NULL)
+ char *new_dirname = malloc (home_len + rest_len + 1);
+ if (new_dirname == NULL)
{
scratch_buffer_free (&pwtmpbuf);
retval = GLOB_NOSPACE;
goto out;
}
+ dirname = new_dirname;
malloc_dirname = 1;
}
d = mempcpy (dirname, p->pw_dir, home_len);
@@ -868,6 +871,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
d = mempcpy (d, end_name, rest_len);
*d = '\0';
+ free (prev_dirname);
+
dirlen = home_len + rest_len;
dirname_modified = 1;
}
--
2.24.1
