From 5f6dda5bdc16028e41f16d1b7c3d9490623796f5 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Mon, 22 Apr 2024 09:07:15 +0100 Subject: [PATCH] gtestutils: Don't follow symlinks when deleting tests' tempdir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when cleaning up the temporary directory tree created by passing G_TEST_OPTION_ISOLATE_DIRS, any symbolic links in that tree would be followed recursively. If the test case has created a symbolic link in its temporary directory to a directory outside that tree, this could lead to unexpected data loss; in particular, if the test case author has (unwisely) created a symbolic link to /, they could lose all data on the system. On systems that have the ftw.h header, replace the current rm_rf() implementation with one that uses nftw() to perform a depth-first traversal (FTW_DEPTH) without following symbolic links (FTW_PHYS), and without crossing mount points (FTW_MOUNT) in case a test has mounted some other filesystem in the temporary directory. The callback logs any error to the standard error stream, but returns 0 rather than -1 to allow nftw() to keep walking the tree rather than terminating immediately. Suppose we are trying to clean up the following tree: tmpdir/ a/ f/ (directory not readable for some reason) g/ p b/ c d Since tmpdir/a/f is not readable, we can expect to fail to delete tmpdir/a/f, tmpdir/a and tmpdir; but it is preferable to (attempt) to delete the rest of the tree rather than failing outright. The cost is that three errors will be logged (for tmpdir/a/f, tmpdir/a and tmpdir). nftw() is part of POSIX.1-2001, SUSv1, and glibc ≥ 2.1, so should be available on effectively every platform except Windows. (And Windows does not enable symbolic links by default so the developer error is less likely to occur there.) The macOS ftw(3) manpage says: > These functions are provided for compatibility with legacy code. New > code should use the fts(3) functions. fts(3) does not seem to be part of any standard, but it does seem to be equally widely supported. The Linux manpages do not indicate that nftw() is deprecated. Fixes: https://gitlab.gnome.org/GNOME/glib/-/issues/3290 --- glib/gtestutils.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++ meson.build | 1 + 2 files changed, 60 insertions(+) diff --git a/glib/gtestutils.c b/glib/gtestutils.c index d241fc952..343d6cc96 100644 --- a/glib/gtestutils.c +++ b/glib/gtestutils.c @@ -30,6 +30,9 @@ #include #include #endif +#ifdef HAVE_FTW_H +#include +#endif #include #include #include @@ -1407,6 +1410,61 @@ parse_args (gint *argc_p, *argc_p = e; } +#ifdef HAVE_FTW_H +static int +rm_rf_nftw_visitor (const char *fpath, + const struct stat *sb, + int typeflag, + struct FTW *ftwbuf) +{ + switch (typeflag) + { + case FTW_DP: + case FTW_D: + case FTW_DNR: + if (g_rmdir (fpath) != 0) + { + int errsv = errno; + g_printerr ("Unable to clean up temporary directory %s: %s\n", + fpath, + g_strerror (errsv)); + } + break; + + default: + if (g_remove (fpath) != 0) + { + int errsv = errno; + g_printerr ("Unable to clean up temporary file %s: %s\n", + fpath, + g_strerror (errsv)); + } + break; + } + + return 0; +} + +static void +rm_rf (const gchar *path) +{ + /* nopenfd specifies the maximum number of directories that [n]ftw() will + * hold open simultaneously. Rather than attempt to determine how many file + * descriptors are available, we assume that 5 are available when tearing + * down a test case; if that assumption is invalid, the only harm is leaving + * a temporary directory on disk. + */ + const int nopenfd = 5; + int ret = nftw (path, rm_rf_nftw_visitor, nopenfd, FTW_DEPTH | FTW_MOUNT | FTW_PHYS); + if (ret != 0) + { + int errsv = errno; + g_printerr ("Unable to clean up temporary directory %s: %s\n", + path, + g_strerror (errsv)); + } +} +#else /* A fairly naive `rm -rf` implementation to clean up after unit tests. */ static void rm_rf (const gchar *path) @@ -1433,6 +1491,7 @@ rm_rf (const gchar *path) g_rmdir (path); } +#endif /* Implement the %G_TEST_OPTION_ISOLATE_DIRS option, iff it’s enabled. Create * a temporary directory for this unit test (disambiguated using @test_run_name) diff --git a/meson.build b/meson.build index 25dc0f2bc..08894dbb0 100644 --- a/meson.build +++ b/meson.build @@ -385,6 +385,7 @@ headers = [ 'dirent.h', # MSC does not come with this by default 'float.h', 'fstab.h', + 'ftw.h', 'grp.h', 'inttypes.h', 'libproc.h',